All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] Makefile: Tidy up of-platdata file generation rules
@ 2021-03-24 17:40 ` Simon Glass
  0 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-24 17:40 UTC (permalink / raw)
  To: u-boot

The recent of-platdata implementation has caused an occasional error in
CI, possibly due to the transition between !OF_PLATDATA_INST and
OF_PLATDATA_INST.

The problems seems to be due to a generated file (C or object file) not
being regenerated when the setting changes.

This series take two steps aimed at correct this problem.

Firstly it makes the set of files generated (in each case) mutually
exclusive, except for the header files which remain common. This means
that the build will fail if new files are not generated when the setting
changes.

Secondly it removes the old generated files before building new ones,
since that could trip things up if the flag changes back again in a
subsequent build.

In addition, dtoc is currently running on every of-platadata build, even
if nothing has changed. Also pylibfdt is always built due to a change in
file naming with Python 3. Both of these problems are fixed.

Changes in v2:
- Adjust tag so this patch doesn't got to dtc list
- Add a comment to the endif
- Only remove the old (and to be unused) files, so avoid confusing make

Simon Glass (6):
  libfdt: Tidy up pylibfdt build rule
  Makefile: Avoid running dtoc every time
  Makefile: Depend only on required of-platdata files
  dtoc: Only generate the required files
  Makefile: Use a variable for generated of-platdata headers
  Makefile: Remove old of-platdata files before regenerating

 scripts/Makefile.spl          | 43 ++++++++++++++++++++++++-----------
 scripts/dtc/pylibfdt/Makefile |  8 +++++--
 tools/dtoc/dtb_platdata.py    | 23 +++++++++++++++----
 tools/dtoc/test_dtoc.py       |  2 +-
 4 files changed, 56 insertions(+), 20 deletions(-)

-- 
2.31.0.291.g576ba9dcdaf-goog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 0/6] Makefile: Tidy up of-platdata file generation rules
@ 2021-03-24 17:40 ` Simon Glass
  0 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-24 17:40 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Masahiro Yamada, Tom Rini, Simon Glass, Andre Przywara, Bin Meng,
	Dalon Westergreen, Devicetree Compiler, Jan Kiszka,
	Masahiro Yamada, Walter Lozano

The recent of-platdata implementation has caused an occasional error in
CI, possibly due to the transition between !OF_PLATDATA_INST and
OF_PLATDATA_INST.

The problems seems to be due to a generated file (C or object file) not
being regenerated when the setting changes.

This series take two steps aimed at correct this problem.

Firstly it makes the set of files generated (in each case) mutually
exclusive, except for the header files which remain common. This means
that the build will fail if new files are not generated when the setting
changes.

Secondly it removes the old generated files before building new ones,
since that could trip things up if the flag changes back again in a
subsequent build.

In addition, dtoc is currently running on every of-platadata build, even
if nothing has changed. Also pylibfdt is always built due to a change in
file naming with Python 3. Both of these problems are fixed.

Changes in v2:
- Adjust tag so this patch doesn't got to dtc list
- Add a comment to the endif
- Only remove the old (and to be unused) files, so avoid confusing make

Simon Glass (6):
  libfdt: Tidy up pylibfdt build rule
  Makefile: Avoid running dtoc every time
  Makefile: Depend only on required of-platdata files
  dtoc: Only generate the required files
  Makefile: Use a variable for generated of-platdata headers
  Makefile: Remove old of-platdata files before regenerating

 scripts/Makefile.spl          | 43 ++++++++++++++++++++++++-----------
 scripts/dtc/pylibfdt/Makefile |  8 +++++--
 tools/dtoc/dtb_platdata.py    | 23 +++++++++++++++----
 tools/dtoc/test_dtoc.py       |  2 +-
 4 files changed, 56 insertions(+), 20 deletions(-)

-- 
2.31.0.291.g576ba9dcdaf-goog


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/6] libfdt: Tidy up pylibfdt build rule
  2021-03-24 17:40 ` Simon Glass
@ 2021-03-24 17:40   ` Simon Glass
  -1 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-24 17:40 UTC (permalink / raw)
  To: u-boot

At present the build rule for pylibfdt depends on _libfdt.so but modern
Python versions add a different suffix to the output file, resulting in
something like _libfdt.cpython-38-x86_64-linux-gnu.so

The result is that pylibfdt is rebuilt every time.

Rename the file the standard name so that the rule works correctly. Also
add libfdt.py to the dependencies, so that file is always created if
missing.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Adjust tag so this patch doesn't got to dtc list

 scripts/dtc/pylibfdt/Makefile | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/scripts/dtc/pylibfdt/Makefile b/scripts/dtc/pylibfdt/Makefile
index 80b6ad2ae71..112198df9de 100644
--- a/scripts/dtc/pylibfdt/Makefile
+++ b/scripts/dtc/pylibfdt/Makefile
@@ -23,12 +23,16 @@ quiet_cmd_pymod = PYMOD   $@
 		SWIG_OPTS="-I$(LIBFDT_srcdir) -I$(LIBFDT_srcdir)/.." \
 		$(PYTHON3) $< --quiet build_ext --inplace
 
-$(obj)/_libfdt.so: $(src)/setup.py $(PYLIBFDT_srcs) FORCE
+$(obj)/_libfdt.so $(obj)/libfdt.py &: $(src)/setup.py $(PYLIBFDT_srcs)
 	@# Remove the library since otherwise Python doesn't seem to regenerate
 	@# the libfdt.py file if it is missing.
 	rm -f $(obj)/_libfdt*.so
 	$(call if_changed,pymod)
+	@# Rename the file to _libfdt.so so this Makefile doesn't run every time
+	@if [ ! -e $(obj)/_libfdt.so ]; then \
+		mv -v $(obj)/_libfdt*.so $(obj)/_libfdt.so; \
+	fi
 
-always += _libfdt.so
+always += _libfdt.so libfdt.py
 
 clean-files += libfdt.i _libfdt.so libfdt.py libfdt_wrap.c
-- 
2.31.0.291.g576ba9dcdaf-goog

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 1/6] libfdt: Tidy up pylibfdt build rule
@ 2021-03-24 17:40   ` Simon Glass
  0 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-24 17:40 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Masahiro Yamada, Tom Rini, Simon Glass, Bin Meng, Devicetree Compiler

At present the build rule for pylibfdt depends on _libfdt.so but modern
Python versions add a different suffix to the output file, resulting in
something like _libfdt.cpython-38-x86_64-linux-gnu.so

The result is that pylibfdt is rebuilt every time.

Rename the file the standard name so that the rule works correctly. Also
add libfdt.py to the dependencies, so that file is always created if
missing.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Adjust tag so this patch doesn't got to dtc list

 scripts/dtc/pylibfdt/Makefile | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/scripts/dtc/pylibfdt/Makefile b/scripts/dtc/pylibfdt/Makefile
index 80b6ad2ae71..112198df9de 100644
--- a/scripts/dtc/pylibfdt/Makefile
+++ b/scripts/dtc/pylibfdt/Makefile
@@ -23,12 +23,16 @@ quiet_cmd_pymod = PYMOD   $@
 		SWIG_OPTS="-I$(LIBFDT_srcdir) -I$(LIBFDT_srcdir)/.." \
 		$(PYTHON3) $< --quiet build_ext --inplace
 
-$(obj)/_libfdt.so: $(src)/setup.py $(PYLIBFDT_srcs) FORCE
+$(obj)/_libfdt.so $(obj)/libfdt.py &: $(src)/setup.py $(PYLIBFDT_srcs)
 	@# Remove the library since otherwise Python doesn't seem to regenerate
 	@# the libfdt.py file if it is missing.
 	rm -f $(obj)/_libfdt*.so
 	$(call if_changed,pymod)
+	@# Rename the file to _libfdt.so so this Makefile doesn't run every time
+	@if [ ! -e $(obj)/_libfdt.so ]; then \
+		mv -v $(obj)/_libfdt*.so $(obj)/_libfdt.so; \
+	fi
 
-always += _libfdt.so
+always += _libfdt.so libfdt.py
 
 clean-files += libfdt.i _libfdt.so libfdt.py libfdt_wrap.c
-- 
2.31.0.291.g576ba9dcdaf-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/6] Makefile: Avoid running dtoc every time
  2021-03-24 17:40 ` Simon Glass
  (?)
  (?)
@ 2021-03-24 17:40 ` Simon Glass
  -1 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-24 17:40 UTC (permalink / raw)
  To: u-boot

Since the dst_dir rule always runs, it causes a rebuild of the of-platdata
files even if not needed.

Create the directory inside the rule instead.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 scripts/Makefile.spl | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 849e9c7060e..4f5876dad95 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -330,12 +330,10 @@ $(obj)/dts/dt-%.o: $(obj)/dts/dt-%.c \
 		include/generated/dt-structs-gen.h prepare FORCE
 	$(call if_changed,plat)
 
-PHONY += dts_dir
-dts_dir:
-	$(shell [ -d $(obj)/dts ] || mkdir -p $(obj)/dts)
-
+# Don't use dts_dir here, since it forces running this expensive rule every time
 include/generated/dt-structs-gen.h $(u-boot-spl-platdata_c) &: \
-		$(obj)/$(SPL_BIN).dtb dts_dir FORCE
+		$(obj)/$(SPL_BIN).dtb
+	@[ -d $(obj)/dts ] || mkdir -p $(obj)/dts
 	$(call if_changed,dtoc)
 
 ifdef CONFIG_SAMSUNG
@@ -476,6 +474,10 @@ FORCE:
 $(obj)/dts/dt-$(SPL_NAME).dtb: dts/dt.dtb
 	$(Q)$(MAKE) $(build)=$(obj)/dts spl_dtbs
 
+PHONY += dts_dir
+dts_dir:
+	$(shell [ -d $(obj)/dts ] || mkdir -p $(obj)/dts)
+
 # Declare the contents of the .PHONY variable as phony.  We keep that
 # information in a variable so we can use it in if_changed and friends.
 .PHONY: $(PHONY)
-- 
2.31.0.291.g576ba9dcdaf-goog

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/6] Makefile: Depend only on required of-platdata files
  2021-03-24 17:40 ` Simon Glass
                   ` (2 preceding siblings ...)
  (?)
@ 2021-03-24 17:40 ` Simon Glass
  -1 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-24 17:40 UTC (permalink / raw)
  To: u-boot

When OF_PLATDATA_INST is enabled, we need dt-uclass.c and dt-device.c for
the build to work. When OF_PLATDATA_INST is not enabled, we only need
dt-plat.c

Update the Makefile rules to indicate this.

At present all files are generated and compiled regardless of which are
actually needed.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 scripts/Makefile.spl | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 4f5876dad95..5f37a82931e 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -120,8 +120,11 @@ endif
 u-boot-spl-init := $(head-y)
 u-boot-spl-main := $(libs-y)
 ifdef CONFIG_$(SPL_TPL_)OF_PLATDATA
-u-boot-spl-platdata := $(obj)/dts/dt-plat.o $(obj)/dts/dt-uclass.o \
-	$(obj)/dts/dt-device.o
+ifdef CONFIG_$(SPL_TPL_)OF_PLATDATA_INST
+u-boot-spl-platdata := $(obj)/dts/dt-uclass.o $(obj)/dts/dt-device.o
+else
+u-boot-spl-platdata := $(obj)/dts/dt-plat.o
+endif
 u-boot-spl-platdata_c := $(patsubst %.o,%.c,$(u-boot-spl-platdata))
 endif
 
-- 
2.31.0.291.g576ba9dcdaf-goog

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/6] dtoc: Only generate the required files
  2021-03-24 17:40 ` Simon Glass
                   ` (3 preceding siblings ...)
  (?)
@ 2021-03-24 17:40 ` Simon Glass
  -1 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-24 17:40 UTC (permalink / raw)
  To: u-boot

At present all possible files are generated, even if some of them just
have a header and an empty body. It is better to generate only the files
that are needed, so that the two types of build (based on the setting of
OF_PLATDATA_INST) can be mutually exclusive.

This is intended to fix a strange problem sometimes found with CI:

   Building current source for 1 boards (1 thread, 40 jobs per thread)
      sandbox:  +   sandbox_spl
   +drivers/built-in.o: In function `dm_setup_inst':
   +drivers/core/root.c:135: undefined reference to
   `_u_boot_list_2_udevice_2_root'
   +dts/dt-uclass.o:(.u_boot_list_2_uclass_2_serial+0x10): undefined
   reference to `_u_boot_list_2_udevice_2_serial'
   ...

This likely happens when switching from !OF_PLATDATA_INST to
OF_PLATDATA_INST since running 'make xxx_defconfig" does not currently
cause any change in which files are generated. With !OF_PLATDATA_INST
the dt-device.c file has no declarations and this is assumed to be the
starting state. The error above seems to indicate that, after changing
to OF_PLATDATA_INST, the dt-uclass.c file is regenerated but the
dt-device.c files is not. This does not seem possible from the relevant
Makefile.spl rule:

   u-boot-spl-platdata := $(obj)/dts/dt-plat.o $(obj)/dts/dt-uclass.o
	$(obj)/dts/dt-device.o

   cmd_dtoc = $(DTOC_ARGS) -c $(obj)/dts -C include/generated all

   include/generated/dt-structs-gen.h $(u-boot-spl-platdata_c) &: \
		$(obj)/$(SPL_BIN).dtb
	@[ -d $(obj)/dts ] || mkdir -p $(obj)/dts
	$(call if_changed,dtoc)

It seems that this cannot regenerate dt-uclass.c without dt-device.c since
'dtoc all' is used. So here the trail ends for now.

In any case it seems better to generate files that are uses and not bother
with those that serve no purpose. So update dtoc to do this automatically.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/dtoc/dtb_platdata.py | 23 +++++++++++++++++++----
 tools/dtoc/test_dtoc.py    |  2 +-
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
index b5c449ebb47..c9c657cb9a9 100644
--- a/tools/dtoc/dtb_platdata.py
+++ b/tools/dtoc/dtb_platdata.py
@@ -1128,7 +1128,7 @@ class DtbPlatdata():
 # Types of output file we understand
 # key: Command used to generate this file
 # value: OutputFile for this command
-OUTPUT_FILES = {
+OUTPUT_FILES_COMMON = {
     'decl':
         OutputFile(Ftype.HEADER, 'dt-decl.h', DtbPlatdata.generate_decl,
                    'Declares externs for all device/uclass instances'),
@@ -1136,9 +1136,17 @@ OUTPUT_FILES = {
         OutputFile(Ftype.HEADER, 'dt-structs-gen.h',
                    DtbPlatdata.generate_structs,
                    'Defines the structs used to hold devicetree data'),
+    }
+
+# File generated without instantiate
+OUTPUT_FILES_NOINST = {
     'platdata':
         OutputFile(Ftype.SOURCE, 'dt-plat.c', DtbPlatdata.generate_plat,
                    'Declares the U_BOOT_DRIVER() records and platform data'),
+    }
+
+# File generated with instantiate
+OUTPUT_FILES_INST = {
     'device':
         OutputFile(Ftype.SOURCE, 'dt-device.c', DtbPlatdata.generate_device,
                    'Declares the DM_DEVICE_INST() records'),
@@ -1204,14 +1212,21 @@ def run_steps(args, dtb_file, include_disabled, output, output_dirs, phase,
     plat.read_aliases()
     plat.assign_seqs()
 
+    # Figure out what output files we plan to generate
+    output_files = OUTPUT_FILES_COMMON
+    if instantiate:
+        output_files.update(OUTPUT_FILES_INST)
+    else:
+        output_files.update(OUTPUT_FILES_NOINST)
+
     cmds = args[0].split(',')
     if 'all' in cmds:
-        cmds = sorted(OUTPUT_FILES.keys())
+        cmds = sorted(output_files.keys())
     for cmd in cmds:
-        outfile = OUTPUT_FILES.get(cmd)
+        outfile = output_files.get(cmd)
         if not outfile:
             raise ValueError("Unknown command '%s': (use: %s)" %
-                             (cmd, ', '.join(sorted(OUTPUT_FILES.keys()))))
+                             (cmd, ', '.join(sorted(output_files.keys()))))
         plat.setup_output(outfile.ftype,
                           outfile.fname if output_dirs else output)
         plat.out_header(outfile)
diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
index 1912a8723f2..e9512834574 100755
--- a/tools/dtoc/test_dtoc.py
+++ b/tools/dtoc/test_dtoc.py
@@ -1554,7 +1554,7 @@ U_BOOT_DRVINFO(spl_test2) = {
         with self.assertRaises(ValueError) as exc:
             self.run_test(['invalid-cmd'], dtb_file, output)
         self.assertIn(
-            "Unknown command 'invalid-cmd': (use: decl, device, platdata, struct, uclass)",
+            "Unknown command 'invalid-cmd': (use: decl, platdata, struct)",
             str(exc.exception))
 
     def test_output_conflict(self):
-- 
2.31.0.291.g576ba9dcdaf-goog

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/6] Makefile: Use a variable for generated of-platdata headers
  2021-03-24 17:40 ` Simon Glass
                   ` (4 preceding siblings ...)
  (?)
@ 2021-03-24 17:40 ` Simon Glass
  -1 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-24 17:40 UTC (permalink / raw)
  To: u-boot

There are actually two generated files but only one is currently mentioned
in the Makefile as a dependency. Put them both in a Makefile variable and
use that instead, to avoid inconsistency.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add a comment to the endif

 scripts/Makefile.spl | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 5f37a82931e..11e9f6a17eb 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -120,13 +120,14 @@ endif
 u-boot-spl-init := $(head-y)
 u-boot-spl-main := $(libs-y)
 ifdef CONFIG_$(SPL_TPL_)OF_PLATDATA
+platdata-hdr := include/generated/dt-structs-gen.h include/generated/dt-decl.h
 ifdef CONFIG_$(SPL_TPL_)OF_PLATDATA_INST
 u-boot-spl-platdata := $(obj)/dts/dt-uclass.o $(obj)/dts/dt-device.o
 else
 u-boot-spl-platdata := $(obj)/dts/dt-plat.o
 endif
 u-boot-spl-platdata_c := $(patsubst %.o,%.c,$(u-boot-spl-platdata))
-endif
+endif  # OF_PLATDATA
 
 # Linker Script
 # First test whether there's a linker-script for the specific stage defined...
@@ -327,15 +328,11 @@ cmd_dtoc = $(DTOC_ARGS) -c $(obj)/dts -C include/generated all
 quiet_cmd_plat = PLAT    $@
 cmd_plat = $(CC) $(c_flags) -c $< -o $(filter-out $(PHONY),$@)
 
-targets += $(u-boot-spl-platdata)
-
-$(obj)/dts/dt-%.o: $(obj)/dts/dt-%.c \
-		include/generated/dt-structs-gen.h prepare FORCE
+$(obj)/dts/dt-%.o: $(obj)/dts/dt-%.c $(platdata-hdr)
 	$(call if_changed,plat)
 
 # Don't use dts_dir here, since it forces running this expensive rule every time
-include/generated/dt-structs-gen.h $(u-boot-spl-platdata_c) &: \
-		$(obj)/$(SPL_BIN).dtb
+$(platdata-hdr) $(u-boot-spl-platdata_c) &: $(obj)/$(SPL_BIN).dtb
 	@[ -d $(obj)/dts ] || mkdir -p $(obj)/dts
 	$(call if_changed,dtoc)
 
-- 
2.31.0.291.g576ba9dcdaf-goog

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/6] Makefile: Remove old of-platdata files before regenerating
  2021-03-24 17:40 ` Simon Glass
                   ` (5 preceding siblings ...)
  (?)
@ 2021-03-24 17:40 ` Simon Glass
  -1 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-24 17:40 UTC (permalink / raw)
  To: u-boot

Which files we generate depends on the setting of OF_PLATDATA_INST in the
build. This might change between builds, but the build directory may be
reused.

Leaving old files around is confusing and switching the OF_PLATDATA_INST
setting does not necessarily regenerate the files, e.g. if the devicetree
has not changed.

Remove all the files before regenerating new ones.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Only remove the old (and to be unused) files, so avoid confusing make

 scripts/Makefile.spl | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 11e9f6a17eb..ca988224dad 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -121,12 +121,22 @@ u-boot-spl-init := $(head-y)
 u-boot-spl-main := $(libs-y)
 ifdef CONFIG_$(SPL_TPL_)OF_PLATDATA
 platdata-hdr := include/generated/dt-structs-gen.h include/generated/dt-decl.h
+platdata-inst := $(obj)/dts/dt-uclass.o $(obj)/dts/dt-device.o
+platdata-noinst := $(obj)/dts/dt-plat.o
+
 ifdef CONFIG_$(SPL_TPL_)OF_PLATDATA_INST
-u-boot-spl-platdata := $(obj)/dts/dt-uclass.o $(obj)/dts/dt-device.o
+u-boot-spl-platdata := $(platdata-inst)
+u-boot-spl-old-platdata := $(platdata-noinst)
 else
-u-boot-spl-platdata := $(obj)/dts/dt-plat.o
+u-boot-spl-platdata := $(platdata-noinst)
+u-boot-spl-old-platdata := $(platdata-inst)
 endif
+
+# Files we need to generate
 u-boot-spl-platdata_c := $(patsubst %.o,%.c,$(u-boot-spl-platdata))
+
+# Files we won't generate and should remove
+u-boot-spl-old-platdata_c := $(patsubst %.o,%.c,$(u-boot-spl-old-platdata))
 endif  # OF_PLATDATA
 
 # Linker Script
@@ -334,6 +344,11 @@ $(obj)/dts/dt-%.o: $(obj)/dts/dt-%.c $(platdata-hdr)
 # Don't use dts_dir here, since it forces running this expensive rule every time
 $(platdata-hdr) $(u-boot-spl-platdata_c) &: $(obj)/$(SPL_BIN).dtb
 	@[ -d $(obj)/dts ] || mkdir -p $(obj)/dts
+	@# Remove old files since which ones we generate depends on the setting
+	@# of OF_PLATDATA_INST and this might change between builds. Leaving old
+	@# ones around is confusing and it is possible that switching the
+	@# setting again will use the old one instead of regenerating it.
+	@rm -f $(u-boot-spl-all-platdata_c) $(u-boot-spl-all-platdata)
 	$(call if_changed,dtoc)
 
 ifdef CONFIG_SAMSUNG
-- 
2.31.0.291.g576ba9dcdaf-goog

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/6] Makefile: Remove old of-platdata files before regenerating
  2021-03-24 17:40 ` Simon Glass
                   ` (6 preceding siblings ...)
  (?)
@ 2021-03-26  3:18 ` Simon Glass
  -1 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-26  3:18 UTC (permalink / raw)
  To: u-boot

Which files we generate depends on the setting of OF_PLATDATA_INST in the
build. This might change between builds, but the build directory may be
reused.

Leaving old files around is confusing and switching the OF_PLATDATA_INST
setting does not necessarily regenerate the files, e.g. if the devicetree
has not changed.

Remove all the files before regenerating new ones.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Only remove the old (and to be unused) files, so avoid confusing make

 scripts/Makefile.spl | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

Applied to u-boot-dm/next, thanks!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 5/6] Makefile: Use a variable for generated of-platdata headers
  2021-03-24 17:40 ` Simon Glass
                   ` (7 preceding siblings ...)
  (?)
@ 2021-03-26  3:18 ` Simon Glass
  -1 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-26  3:18 UTC (permalink / raw)
  To: u-boot

There are actually two generated files but only one is currently mentioned
in the Makefile as a dependency. Put them both in a Makefile variable and
use that instead, to avoid inconsistency.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add a comment to the endif

 scripts/Makefile.spl | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

Applied to u-boot-dm/next, thanks!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 4/6] dtoc: Only generate the required files
  2021-03-24 17:40 ` Simon Glass
                   ` (8 preceding siblings ...)
  (?)
@ 2021-03-26  3:18 ` Simon Glass
  -1 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-26  3:18 UTC (permalink / raw)
  To: u-boot

At present all possible files are generated, even if some of them just
have a header and an empty body. It is better to generate only the files
that are needed, so that the two types of build (based on the setting of
OF_PLATDATA_INST) can be mutually exclusive.

This is intended to fix a strange problem sometimes found with CI:

   Building current source for 1 boards (1 thread, 40 jobs per thread)
      sandbox:  +   sandbox_spl
   +drivers/built-in.o: In function `dm_setup_inst':
   +drivers/core/root.c:135: undefined reference to
   `_u_boot_list_2_udevice_2_root'
   +dts/dt-uclass.o:(.u_boot_list_2_uclass_2_serial+0x10): undefined
   reference to `_u_boot_list_2_udevice_2_serial'
   ...

This likely happens when switching from !OF_PLATDATA_INST to
OF_PLATDATA_INST since running 'make xxx_defconfig" does not currently
cause any change in which files are generated. With !OF_PLATDATA_INST
the dt-device.c file has no declarations and this is assumed to be the
starting state. The error above seems to indicate that, after changing
to OF_PLATDATA_INST, the dt-uclass.c file is regenerated but the
dt-device.c files is not. This does not seem possible from the relevant
Makefile.spl rule:

   u-boot-spl-platdata := $(obj)/dts/dt-plat.o $(obj)/dts/dt-uclass.o
	$(obj)/dts/dt-device.o

   cmd_dtoc = $(DTOC_ARGS) -c $(obj)/dts -C include/generated all

   include/generated/dt-structs-gen.h $(u-boot-spl-platdata_c) &: \
		$(obj)/$(SPL_BIN).dtb
	@[ -d $(obj)/dts ] || mkdir -p $(obj)/dts
	$(call if_changed,dtoc)

It seems that this cannot regenerate dt-uclass.c without dt-device.c since
'dtoc all' is used. So here the trail ends for now.

In any case it seems better to generate files that are uses and not bother
with those that serve no purpose. So update dtoc to do this automatically.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/dtoc/dtb_platdata.py | 23 +++++++++++++++++++----
 tools/dtoc/test_dtoc.py    |  2 +-
 2 files changed, 20 insertions(+), 5 deletions(-)

Applied to u-boot-dm/next, thanks!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 3/6] Makefile: Depend only on required of-platdata files
  2021-03-24 17:40 ` Simon Glass
                   ` (9 preceding siblings ...)
  (?)
@ 2021-03-26  3:18 ` Simon Glass
  -1 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-26  3:18 UTC (permalink / raw)
  To: u-boot

When OF_PLATDATA_INST is enabled, we need dt-uclass.c and dt-device.c for
the build to work. When OF_PLATDATA_INST is not enabled, we only need
dt-plat.c

Update the Makefile rules to indicate this.

At present all files are generated and compiled regardless of which are
actually needed.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 scripts/Makefile.spl | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Applied to u-boot-dm/next, thanks!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 2/6] Makefile: Avoid running dtoc every time
  2021-03-24 17:40 ` Simon Glass
                   ` (10 preceding siblings ...)
  (?)
@ 2021-03-26  3:18 ` Simon Glass
  -1 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-26  3:18 UTC (permalink / raw)
  To: u-boot

Since the dst_dir rule always runs, it causes a rebuild of the of-platdata
files even if not needed.

Create the directory inside the rule instead.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 scripts/Makefile.spl | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Applied to u-boot-dm/next, thanks!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/6] libfdt: Tidy up pylibfdt build rule
@ 2021-03-26  3:18   ` Simon Glass
  0 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-26  3:18 UTC (permalink / raw)
  To: u-boot

At present the build rule for pylibfdt depends on _libfdt.so but modern
Python versions add a different suffix to the output file, resulting in
something like _libfdt.cpython-38-x86_64-linux-gnu.so

The result is that pylibfdt is rebuilt every time.

Rename the file the standard name so that the rule works correctly. Also
add libfdt.py to the dependencies, so that file is always created if
missing.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Adjust tag so this patch doesn't got to dtc list

 scripts/dtc/pylibfdt/Makefile | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Applied to u-boot-dm/next, thanks!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/6] libfdt: Tidy up pylibfdt build rule
@ 2021-03-26  3:18   ` Simon Glass
  0 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2021-03-26  3:18 UTC (permalink / raw)
  To: Simon Glass
  Cc: Masahiro Yamada, Tom Rini, Bin Meng, Devicetree Compiler,
	U-Boot Mailing List

At present the build rule for pylibfdt depends on _libfdt.so but modern
Python versions add a different suffix to the output file, resulting in
something like _libfdt.cpython-38-x86_64-linux-gnu.so

The result is that pylibfdt is rebuilt every time.

Rename the file the standard name so that the rule works correctly. Also
add libfdt.py to the dependencies, so that file is always created if
missing.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

Changes in v2:
- Adjust tag so this patch doesn't got to dtc list

 scripts/dtc/pylibfdt/Makefile | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Applied to u-boot-dm/next, thanks!

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-03-26  3:18 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-24 17:40 [PATCH v2 0/6] Makefile: Tidy up of-platdata file generation rules Simon Glass
2021-03-24 17:40 ` Simon Glass
2021-03-24 17:40 ` [PATCH v2 1/6] libfdt: Tidy up pylibfdt build rule Simon Glass
2021-03-24 17:40   ` Simon Glass
2021-03-24 17:40 ` [PATCH v2 2/6] Makefile: Avoid running dtoc every time Simon Glass
2021-03-24 17:40 ` [PATCH v2 3/6] Makefile: Depend only on required of-platdata files Simon Glass
2021-03-24 17:40 ` [PATCH v2 4/6] dtoc: Only generate the required files Simon Glass
2021-03-24 17:40 ` [PATCH v2 5/6] Makefile: Use a variable for generated of-platdata headers Simon Glass
2021-03-24 17:40 ` [PATCH v2 6/6] Makefile: Remove old of-platdata files before regenerating Simon Glass
2021-03-26  3:18 ` Simon Glass
2021-03-26  3:18 ` [PATCH v2 5/6] Makefile: Use a variable for generated of-platdata headers Simon Glass
2021-03-26  3:18 ` [PATCH v2 4/6] dtoc: Only generate the required files Simon Glass
2021-03-26  3:18 ` [PATCH v2 3/6] Makefile: Depend only on required of-platdata files Simon Glass
2021-03-26  3:18 ` [PATCH v2 2/6] Makefile: Avoid running dtoc every time Simon Glass
2021-03-26  3:18 ` [PATCH v2 1/6] libfdt: Tidy up pylibfdt build rule Simon Glass
2021-03-26  3:18   ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.