Re: [U-Boot] [PATCH] spl: unbreak CONFIG_SPL_MULTI_DTB_FIT after fixing CONFIG_OF_EMBED
On Wed, 10/01/18 09:48, Lokesh Vutla wrote:
On Wednesday 10 January 2018 12:32 PM, Goldschmidt Simon wrote:
Commit 9bd76b80 "spl: make CONFIG_OF_EMBED pass dts through fdtgrep" moved the fdtgrep code from scripts/Makefile.spl to dts/Makefile so that the dtb is stripped in embedded mode, too.
This broke CONFIG_SPL_MULTI_DTB_FIT where fdtgrep is still called from scripts/Makefile.spl to strip all dtbs in CONFIG_SPL_OF_LIST. To fix that, cmd_fdtgrep is brought back into scripts/Makefile.spl at the downside of having it duplicated in two makefiles.
Missing Signed-off-by? Also a Reported-by credits would be nice :)
Right. I'll add that in a next version, sorry.
scripts/Makefile.spl | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 64390e5785..72e74f14c3 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -239,6 +239,22 @@ $(obj)/$(SPL_BIN)-pad.bin: $(obj)/$(SPL_BIN) @bss_size_str=$(shell $(NM) $< | awk 'BEGIN {size = 0} /__bss_size/ {size
= $$1} END {print "ibase=16; " toupper(size)}' | bc); \
dd if=/dev/zero of=$@ bs=1 count=$${bss_size_str} 2>/dev/null;
+# Pass the original device tree file through fdtgrep twice. The first +pass # removes any unwanted nodes (i.e. those which don't have the # +'u-boot,dm-pre-reloc' property and thus are not needed by SPL. The +second # pass removes various unused properties from the remaining nodes. +# The output is typically a much smaller device tree file. +ifeq ($(CONFIG_TPL_BUILD),y) +fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-tpl else +fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-spl endif +quiet_cmd_fdtgrep = FDTGREP $@
cmd_fdtgrep = $(objtree)/tools/fdtgrep $(fdtgrep_props) -RT $< \-n /chosen -n /config -O dtb | \- $(objtree)/tools/fdtgrep -r -O dtb - -o $@ \
$(addprefix -P ,$(subst$",,$(CONFIG_OF_SPL_REMOVE_PROPS)))
hmm..we are duplicating code here. Why can't dtb build for CONFIG_OF_EMBED be moved to scripts/Makefile.spl?
I don't like the code duplication either. Back in November, I tried to create the fdtgrep'ed ftd from Makefile.spl but failed. Seems I'm not a pro in makefiles... Of course I'd appreciate it if you'd find a solution for that!
Regards, Simon
On Wednesday 10 January 2018 07:40 PM, Goldschmidt Simon wrote:
On Wed, 10/01/18 09:48, Lokesh Vutla wrote:
On Wednesday 10 January 2018 12:32 PM, Goldschmidt Simon wrote:
Commit 9bd76b80 "spl: make CONFIG_OF_EMBED pass dts through fdtgrep" moved the fdtgrep code from scripts/Makefile.spl to dts/Makefile so that the dtb is stripped in embedded mode, too.
This broke CONFIG_SPL_MULTI_DTB_FIT where fdtgrep is still called from scripts/Makefile.spl to strip all dtbs in CONFIG_SPL_OF_LIST. To fix that, cmd_fdtgrep is brought back into scripts/Makefile.spl at the downside of having it duplicated in two makefiles.
Missing Signed-off-by? Also a Reported-by credits would be nice :)
Right. I'll add that in a next version, sorry.
scripts/Makefile.spl | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 64390e5785..72e74f14c3 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -239,6 +239,22 @@ $(obj)/$(SPL_BIN)-pad.bin: $(obj)/$(SPL_BIN) @bss_size_str=$(shell $(NM) $< | awk 'BEGIN {size = 0} /__bss_size/ {size
= $$1} END {print "ibase=16; " toupper(size)}' | bc); \
dd if=/dev/zero of=$@ bs=1 count=$${bss_size_str} 2>/dev/null;
+# Pass the original device tree file through fdtgrep twice. The first +pass # removes any unwanted nodes (i.e. those which don't have the # +'u-boot,dm-pre-reloc' property and thus are not needed by SPL. The +second # pass removes various unused properties from the remaining nodes. +# The output is typically a much smaller device tree file. +ifeq ($(CONFIG_TPL_BUILD),y) +fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-tpl else +fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-spl endif +quiet_cmd_fdtgrep = FDTGREP $@
cmd_fdtgrep = $(objtree)/tools/fdtgrep $(fdtgrep_props) -RT $< \-n /chosen -n /config -O dtb | \- $(objtree)/tools/fdtgrep -r -O dtb - -o $@ \
$(addprefix -P ,$(subst$",,$(CONFIG_OF_SPL_REMOVE_PROPS)))
hmm..we are duplicating code here. Why can't dtb build for CONFIG_OF_EMBED be moved to scripts/Makefile.spl?
I don't like the code duplication either. Back in November, I tried to create the fdtgrep'ed ftd from Makefile.spl but failed. Seems I'm not a pro in makefiles... Of course I'd appreciate it if you'd find a solution for that!
Simon, Tom, Any suggestions? Should we shift entire spl-dtb support to dts/Makefile?
Thanks and regards, Lokesh
Regards, Simon
Hi Lokesh,
On 11 January 2018 at 00:33, Lokesh Vutla lokeshvutla@ti.com wrote:
On Wednesday 10 January 2018 07:40 PM, Goldschmidt Simon wrote:
On Wed, 10/01/18 09:48, Lokesh Vutla wrote:
On Wednesday 10 January 2018 12:32 PM, Goldschmidt Simon wrote:
Commit 9bd76b80 "spl: make CONFIG_OF_EMBED pass dts through fdtgrep" moved the fdtgrep code from scripts/Makefile.spl to dts/Makefile so that the dtb is stripped in embedded mode, too.
This broke CONFIG_SPL_MULTI_DTB_FIT where fdtgrep is still called from scripts/Makefile.spl to strip all dtbs in CONFIG_SPL_OF_LIST. To fix that, cmd_fdtgrep is brought back into scripts/Makefile.spl at the downside of having it duplicated in two makefiles.
Missing Signed-off-by? Also a Reported-by credits would be nice :)
Right. I'll add that in a next version, sorry.
scripts/Makefile.spl | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 64390e5785..72e74f14c3 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -239,6 +239,22 @@ $(obj)/$(SPL_BIN)-pad.bin: $(obj)/$(SPL_BIN) @bss_size_str=$(shell $(NM) $< | awk 'BEGIN {size = 0} /__bss_size/ {size
= $$1} END {print "ibase=16; " toupper(size)}' | bc); \
dd if=/dev/zero of=$@ bs=1 count=$${bss_size_str} 2>/dev/null;+# Pass the original device tree file through fdtgrep twice. The first +pass # removes any unwanted nodes (i.e. those which don't have the # +'u-boot,dm-pre-reloc' property and thus are not needed by SPL. The +second # pass removes various unused properties from the remaining nodes. +# The output is typically a much smaller device tree file. +ifeq ($(CONFIG_TPL_BUILD),y) +fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-tpl else +fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-spl endif +quiet_cmd_fdtgrep = FDTGREP $@
cmd_fdtgrep = $(objtree)/tools/fdtgrep $(fdtgrep_props) -RT $< \-n /chosen -n /config -O dtb | \- $(objtree)/tools/fdtgrep -r -O dtb - -o $@ \
$(addprefix -P ,$(subst$",,$(CONFIG_OF_SPL_REMOVE_PROPS)))
hmm..we are duplicating code here. Why can't dtb build for CONFIG_OF_EMBED be moved to scripts/Makefile.spl?
I don't like the code duplication either. Back in November, I tried to create the fdtgrep'ed ftd from Makefile.spl but failed. Seems I'm not a pro in makefiles... Of course I'd appreciate it if you'd find a solution for that!
Simon, Tom, Any suggestions? Should we shift entire spl-dtb support to dts/Makefile?
We do try to keep logic out of the Makefiles in source-file subdirs, reserving it for the main Makefile and those in scripts/.But dts/Makefile is a bit of a special case, since there are no source files there. I don't have a strong opinion either way.
Thanks and regards, Lokesh
Regards, Simon
On Thu, Jan 11, 2018 at 07:55:03AM -0800, Simon Glass wrote:
Hi Lokesh,
On 11 January 2018 at 00:33, Lokesh Vutla lokeshvutla@ti.com wrote:
On Wednesday 10 January 2018 07:40 PM, Goldschmidt Simon wrote:
On Wed, 10/01/18 09:48, Lokesh Vutla wrote:
On Wednesday 10 January 2018 12:32 PM, Goldschmidt Simon wrote:
Commit 9bd76b80 "spl: make CONFIG_OF_EMBED pass dts through fdtgrep" moved the fdtgrep code from scripts/Makefile.spl to dts/Makefile so that the dtb is stripped in embedded mode, too.
This broke CONFIG_SPL_MULTI_DTB_FIT where fdtgrep is still called from scripts/Makefile.spl to strip all dtbs in CONFIG_SPL_OF_LIST. To fix that, cmd_fdtgrep is brought back into scripts/Makefile.spl at the downside of having it duplicated in two makefiles.
Missing Signed-off-by? Also a Reported-by credits would be nice :)
Right. I'll add that in a next version, sorry.
scripts/Makefile.spl | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 64390e5785..72e74f14c3 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -239,6 +239,22 @@ $(obj)/$(SPL_BIN)-pad.bin: $(obj)/$(SPL_BIN) @bss_size_str=$(shell $(NM) $< | awk 'BEGIN {size = 0} /__bss_size/ {size
= $$1} END {print "ibase=16; " toupper(size)}' | bc); \
dd if=/dev/zero of=$@ bs=1 count=$${bss_size_str} 2>/dev/null;+# Pass the original device tree file through fdtgrep twice. The first +pass # removes any unwanted nodes (i.e. those which don't have the # +'u-boot,dm-pre-reloc' property and thus are not needed by SPL. The +second # pass removes various unused properties from the remaining nodes. +# The output is typically a much smaller device tree file. +ifeq ($(CONFIG_TPL_BUILD),y) +fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-tpl else +fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-spl endif +quiet_cmd_fdtgrep = FDTGREP $@
cmd_fdtgrep = $(objtree)/tools/fdtgrep $(fdtgrep_props) -RT $< \-n /chosen -n /config -O dtb | \- $(objtree)/tools/fdtgrep -r -O dtb - -o $@ \
$(addprefix -P ,$(subst$",,$(CONFIG_OF_SPL_REMOVE_PROPS)))
hmm..we are duplicating code here. Why can't dtb build for CONFIG_OF_EMBED be moved to scripts/Makefile.spl?
I don't like the code duplication either. Back in November, I tried to create the fdtgrep'ed ftd from Makefile.spl but failed. Seems I'm not a pro in makefiles... Of course I'd appreciate it if you'd find a solution for that!
Simon, Tom, Any suggestions? Should we shift entire spl-dtb support to dts/Makefile?
We do try to keep logic out of the Makefiles in source-file subdirs, reserving it for the main Makefile and those in scripts/.But dts/Makefile is a bit of a special case, since there are no source files there. I don't have a strong opinion either way.
Welp, lets try shifting it then and see if things look cleaner overall then, thanks!
On 11.01.2018 17:20, Tom Rini wrote:
On Thu, Jan 11, 2018 at 07:55:03AM -0800, Simon Glass wrote:
Hi Lokesh,
On 11 January 2018 at 00:33, Lokesh Vutla lokeshvutla@ti.com wrote:
On Wednesday 10 January 2018 07:40 PM, Goldschmidt Simon wrote:
On Wed, 10/01/18 09:48, Lokesh Vutla wrote:
On Wednesday 10 January 2018 12:32 PM, Goldschmidt Simon wrote:
Commit 9bd76b80 "spl: make CONFIG_OF_EMBED pass dts through fdtgrep" moved the fdtgrep code from scripts/Makefile.spl to dts/Makefile so that the dtb is stripped in embedded mode, too.
This broke CONFIG_SPL_MULTI_DTB_FIT where fdtgrep is still called from scripts/Makefile.spl to strip all dtbs in CONFIG_SPL_OF_LIST. To fix that, cmd_fdtgrep is brought back into scripts/Makefile.spl at the downside of having it duplicated in two makefiles.
Missing Signed-off-by? Also a Reported-by credits would be nice :)
Right. I'll add that in a next version, sorry.
scripts/Makefile.spl | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 64390e5785..72e74f14c3 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -239,6 +239,22 @@ $(obj)/$(SPL_BIN)-pad.bin: $(obj)/$(SPL_BIN) @bss_size_str=$(shell $(NM) $< | awk 'BEGIN {size = 0} /__bss_size/ {size
= $$1} END {print "ibase=16; " toupper(size)}' | bc); \
dd if=/dev/zero of=$@ bs=1 count=$${bss_size_str} 2>/dev/null;+# Pass the original device tree file through fdtgrep twice. The first +pass # removes any unwanted nodes (i.e. those which don't have the # +'u-boot,dm-pre-reloc' property and thus are not needed by SPL. The +second # pass removes various unused properties from the remaining nodes. +# The output is typically a much smaller device tree file. +ifeq ($(CONFIG_TPL_BUILD),y) +fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-tpl else +fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-spl endif +quiet_cmd_fdtgrep = FDTGREP $@
cmd_fdtgrep = $(objtree)/tools/fdtgrep $(fdtgrep_props) -RT $< \-n /chosen -n /config -O dtb | \- $(objtree)/tools/fdtgrep -r -O dtb - -o $@ \
$(addprefix -P ,$(subst$",,$(CONFIG_OF_SPL_REMOVE_PROPS)))
hmm..we are duplicating code here. Why can't dtb build for CONFIG_OF_EMBED be moved to scripts/Makefile.spl?
I don't like the code duplication either. Back in November, I tried to create the fdtgrep'ed ftd from Makefile.spl but failed. Seems I'm not a pro in makefiles... Of course I'd appreciate it if you'd find a solution for that!
Simon, Tom, Any suggestions? Should we shift entire spl-dtb support to dts/Makefile?
We do try to keep logic out of the Makefiles in source-file subdirs, reserving it for the main Makefile and those in scripts/.But dts/Makefile is a bit of a special case, since there are no source files there. I don't have a strong opinion either way.
Welp, lets try shifting it then and see if things look cleaner overall then, thanks!
Hmm, I don't know where that would end. I like the idea of keeping spl-dtb support in dts/Makefile.
Would it make sense to move the definition of fdtgrep to a common place instead? Do we have one?
Simon
participants (5)
-
Goldschmidt Simon -
Lokesh Vutla -
Simon Glass -
Simon Goldschmidt
-
Tom Rini