* [Buildroot] [PATCH 0/3] linux: Fix appended DTB image generation
@ 2015-10-18 21:04 Thomas Petazzoni
2015-10-18 21:05 ` [Buildroot] [PATCH 1/3] linux: de-duplicate DTB and Linux image installation Thomas Petazzoni
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2015-10-18 21:04 UTC (permalink / raw)
To: buildroot
Hello,
What initially prompted this short series is the oldest patch
currently sitting in patchwork, i.e
http://patchwork.ozlabs.org/patch/394002/. In this patch, Guido
Mart?nez wanted to fix a problem in our implementation of the appended
DTB image generation: we are concatenating over and over again the DTB
at the end of the same file, so that if you do "make linux-rebuild"
multiple times, your kernel image ends up having your DTB concatenated
multiple times to it. This is because we do the appending "in place",
directly on zImage.
To solve this problem, this series moves to generating the appended
image under a different name: zImage.<dtb> (or uImage.<dtb> if the
uImage format is used).
But since this naming allows to generate *multiple* appended DTB
images (which was forbidden until now), we take this opportunity to
make such a thing possible.
This is all done in the last patch of this series, the two first
patches being merely preparation work for the last patch.
Please help get rid of the oldest patch in patchwork by
reviewing/testing this series!
Best regards,
Thomas
Thomas Petazzoni (3):
linux: de-duplicate DTB and Linux image installation
linux: only install the DTBs when not in appended DTB mode
linux: don't build appended DTB image in place and support multiple
images
linux/linux.mk | 70 ++++++++++++++++++++++++++++++++--------------------------
1 file changed, 39 insertions(+), 31 deletions(-)
--
2.6.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/3] linux: de-duplicate DTB and Linux image installation
2015-10-18 21:04 [Buildroot] [PATCH 0/3] linux: Fix appended DTB image generation Thomas Petazzoni
@ 2015-10-18 21:05 ` Thomas Petazzoni
2015-10-18 21:05 ` [Buildroot] [PATCH 2/3] linux: only install the DTBs when not in appended DTB mode Thomas Petazzoni
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2015-10-18 21:05 UTC (permalink / raw)
To: buildroot
Currently, the LINUX_INSTALL_DTB and LINUX_INSTALL_DTB_TARGET macros
are exactly the same, except for the target directory.
Similarly, LINUX_INSTALL_KERNEL_IMAGE_TO_TARGET and
LINUX_INSTALL_IMAGES_CMDS are copying the kernel image, just to a
different place (and with a different strategy).
As a preparation for future additions, this commit de-duplicate this
code:
- LINUX_INSTALL_DTB becomes a make macro that takes one argument: the
destination directory.
- LINUX_INSTALL_IMAGE is a new make macro that also takes on
argument: the destination directory.
Both macros are used by LINUX_INSTALL_KERNEL_IMAGE_TO_TARGET and
LINUX_INSTALL_IMAGES_CMDS to respectively install to the target
directory and the images directory.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
linux/linux.mk | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/linux/linux.mk b/linux/linux.mk
index 6c7ecfc..149bd69 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -236,14 +236,7 @@ define LINUX_INSTALL_DTB
cp $(addprefix \
$(KERNEL_ARCH_PATH)/boot/$(if $(wildcard \
$(addprefix $(KERNEL_ARCH_PATH)/boot/dts/,$(KERNEL_DTBS))),dts/),$(KERNEL_DTBS)) \
- $(BINARIES_DIR)/
-endef
-define LINUX_INSTALL_DTB_TARGET
- # dtbs moved from arch/<ARCH>/boot to arch/<ARCH>/boot/dts since 3.8-rc1
- cp $(addprefix \
- $(KERNEL_ARCH_PATH)/boot/$(if $(wildcard \
- $(addprefix $(KERNEL_ARCH_PATH)/boot/dts/,$(KERNEL_DTBS))),dts/),$(KERNEL_DTBS)) \
- $(TARGET_DIR)/boot/
+ $(1)
endef
endif
endif
@@ -284,11 +277,14 @@ define LINUX_BUILD_CMDS
$(LINUX_APPEND_DTB)
endef
+define LINUX_INSTALL_IMAGE
+ $(INSTALL) -m 0644 -D $(LINUX_IMAGE_PATH) $(1)/$(LINUX_IMAGE_NAME)
+endef
ifeq ($(BR2_LINUX_KERNEL_INSTALL_TARGET),y)
define LINUX_INSTALL_KERNEL_IMAGE_TO_TARGET
- install -m 0644 -D $(LINUX_IMAGE_PATH) $(TARGET_DIR)/boot/$(LINUX_IMAGE_NAME)
- $(LINUX_INSTALL_DTB_TARGET)
+ $(call LINUX_INSTALL_IMAGE,$(TARGET_DIR)/boot)
+ $(call LINUX_INSTALL_DTB,$(TARGET_DIR)/boot)
endef
endif
@@ -302,8 +298,8 @@ endef
define LINUX_INSTALL_IMAGES_CMDS
- cp $(LINUX_IMAGE_PATH) $(BINARIES_DIR)
- $(LINUX_INSTALL_DTB)
+ $(call LINUX_INSTALL_IMAGE,$(BINARIES_DIR))
+ $(call LINUX_INSTALL_DTB,$(BINARIES_DIR))
endef
define LINUX_INSTALL_TARGET_CMDS
--
2.6.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 2/3] linux: only install the DTBs when not in appended DTB mode
2015-10-18 21:04 [Buildroot] [PATCH 0/3] linux: Fix appended DTB image generation Thomas Petazzoni
2015-10-18 21:05 ` [Buildroot] [PATCH 1/3] linux: de-duplicate DTB and Linux image installation Thomas Petazzoni
@ 2015-10-18 21:05 ` Thomas Petazzoni
2015-10-18 21:05 ` [Buildroot] [PATCH 3/3] linux: don't build appended DTB image in place and support multiple images Thomas Petazzoni
2015-12-20 14:26 ` [Buildroot] [PATCH 0/3] linux: Fix appended DTB image generation Thomas Petazzoni
3 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2015-10-18 21:05 UTC (permalink / raw)
To: buildroot
When you're using the "appended DTB" mode, the Device Tree blob gets
appended to your kernel image, so there is no point in installing both
the DTB and the kernel image to the images or target directories,
installing the kernel image itself is sufficient.
Therefore, this commit disables the definition of LINUX_INSTALL_DTB
when appended DTB is used.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
linux/linux.mk | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/linux/linux.mk b/linux/linux.mk
index 149bd69..59e1bb4 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -231,6 +231,7 @@ ifeq ($(BR2_LINUX_KERNEL_DTB_IS_SELF_BUILT),)
define LINUX_BUILD_DTB
$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(KERNEL_DTBS)
endef
+ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB),)
define LINUX_INSTALL_DTB
# dtbs moved from arch/<ARCH>/boot to arch/<ARCH>/boot/dts since 3.8-rc1
cp $(addprefix \
@@ -238,8 +239,9 @@ define LINUX_INSTALL_DTB
$(addprefix $(KERNEL_ARCH_PATH)/boot/dts/,$(KERNEL_DTBS))),dts/),$(KERNEL_DTBS)) \
$(1)
endef
-endif
-endif
+endif # BR2_LINUX_KERNEL_APPENDED_DTB
+endif # BR2_LINUX_KERNEL_DTB_IS_SELF_BUILT
+endif # BR2_LINUX_KERNEL_DTS_SUPPORT
ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB),y)
# dtbs moved from arch/$ARCH/boot to arch/$ARCH/boot/dts since 3.8-rc1
--
2.6.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 3/3] linux: don't build appended DTB image in place and support multiple images
2015-10-18 21:04 [Buildroot] [PATCH 0/3] linux: Fix appended DTB image generation Thomas Petazzoni
2015-10-18 21:05 ` [Buildroot] [PATCH 1/3] linux: de-duplicate DTB and Linux image installation Thomas Petazzoni
2015-10-18 21:05 ` [Buildroot] [PATCH 2/3] linux: only install the DTBs when not in appended DTB mode Thomas Petazzoni
@ 2015-10-18 21:05 ` Thomas Petazzoni
2015-10-20 5:22 ` Peter Korsgaard
2015-12-20 14:26 ` [Buildroot] [PATCH 0/3] linux: Fix appended DTB image generation Thomas Petazzoni
3 siblings, 1 reply; 6+ messages in thread
From: Thomas Petazzoni @ 2015-10-18 21:05 UTC (permalink / raw)
To: buildroot
Currently, the linux.mk logic for appended DTB image does the
appending of the DTB in place, directly at the end of the zImage using
a >> sign. This is incorrect because if you run "make linux-rebuild"
multiple times, you get the DTB appended over and over again to the
image.
Since keeping the 'zImage' or 'uImage' name for the appended DTB image
is not very clear, this commit moves to using the 'zImage.<dtb>' and
'uImage.<dtb>' format. This way, we can clearly distinguish the
original image from the appended one.
In addition, this naming scheme easily allows to generate *multiple*
appended DTB images: from one zImage, you can generate multiple
zImage.<dtb> for several DTBs, and then generate (if requested) the
corresponding uImage.<dtb>.
To achieve this, this commit:
- Changes the definition of LINUX_APPENDED_DTB to iterate over
$(KERNEL_DTS_NAME), and generate a zImage.<dtb> image for each of
them.
- Changes the addition of LINUX_APPENDED_DTB for appended uImage to
also iterate over $(KERNEL_DTS_NAME).
- Provide a different implementation of LINUX_INSTALL_IMAGE which
installs all the appended DTB images (but not the bare image)
- Remove the checks that verified that only one DT name is passed
when appended DTB is used, since we now support generating multiple
DT images.
Some of the tested configuration:
- Normal uImage with several DTBs
BR2_LINUX_KERNEL_DEFCONFIG="mvebu_v7"
BR2_LINUX_KERNEL_UIMAGE=y
BR2_LINUX_KERNEL_UIMAGE_LOADADDR="0x200000"
BR2_LINUX_KERNEL_DTS_SUPPORT=y
BR2_LINUX_KERNEL_INTREE_DTS_NAME="armada-xp-matrix armada-xp-gp armada-370-mirabox"
Contents of output/images/:
armada-370-mirabox.dtb armada-xp-gp.dtb armada-xp-matrix.dtb uImage
- Normal zImage with several DTBs
BR2_LINUX_KERNEL_DEFCONFIG="mvebu_v7"
BR2_LINUX_KERNEL_ZIMAGE=y
BR2_LINUX_KERNEL_DTS_SUPPORT=y
BR2_LINUX_KERNEL_INTREE_DTS_NAME="armada-xp-matrix armada-xp-gp armada-370-mirabox"
Contents of output/images:
armada-370-mirabox.dtb armada-xp-gp.dtb armada-xp-matrix.dtb zImage
- Appended uImage with several DTBs:
BR2_LINUX_KERNEL_DEFCONFIG="mvebu_v7"
BR2_LINUX_KERNEL_APPENDED_UIMAGE=y
BR2_LINUX_KERNEL_UIMAGE_LOADADDR="0x200000"
BR2_LINUX_KERNEL_INTREE_DTS_NAME="armada-xp-matrix armada-xp-gp armada-370-mirabox"
Contents of output/images:
uImage.armada-370-mirabox uImage.armada-xp-gp uImage.armada-xp-matrix
- Appended zImage with several DTBs:
BR2_LINUX_KERNEL_DEFCONFIG="mvebu_v7"
BR2_LINUX_KERNEL_APPENDED_ZIMAGE=y
BR2_LINUX_KERNEL_INTREE_DTS_NAME="armada-xp-matrix armada-xp-gp armada-370-mirabox"
Contents of output/images:
zImage.armada-370-mirabox zImage.armada-xp-gp zImage.armada-xp-matrix
In all configurations, the contents of output/target/boot/ was the
same if BR2_LINUX_KERNEL_INSTALL_TARGET=y.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
linux/linux.mk | 44 +++++++++++++++++++++++++++-----------------
1 file changed, 27 insertions(+), 17 deletions(-)
diff --git a/linux/linux.mk b/linux/linux.mk
index 59e1bb4..297ef83 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -246,11 +246,15 @@ endif # BR2_LINUX_KERNEL_DTS_SUPPORT
ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB),y)
# dtbs moved from arch/$ARCH/boot to arch/$ARCH/boot/dts since 3.8-rc1
define LINUX_APPEND_DTB
- if [ -e $(KERNEL_ARCH_PATH)/boot/$(KERNEL_DTS_NAME).dtb ]; then \
- cat $(KERNEL_ARCH_PATH)/boot/$(KERNEL_DTS_NAME).dtb; \
- else \
- cat $(KERNEL_ARCH_PATH)/boot/dts/$(KERNEL_DTS_NAME).dtb; \
- fi >> $(KERNEL_ARCH_PATH)/boot/zImage
+ (cd $(KERNEL_ARCH_PATH)/boot; \
+ for dtb in $(KERNEL_DTS_NAME); do \
+ if test -e $${dtb}.dtb ; then \
+ dtbpath=$${dtb}.dtb ; \
+ else \
+ dtbpath=dts/$${dtb}.dtb ; \
+ fi ; \
+ cat zImage $${dtbpath} > zImage.$${dtb} || exit 1; \
+ done)
endef
ifeq ($(BR2_LINUX_KERNEL_APPENDED_UIMAGE),y)
# We need to generate a new u-boot image that takes into
@@ -258,11 +262,14 @@ ifeq ($(BR2_LINUX_KERNEL_APPENDED_UIMAGE),y)
# of the image. To do so, we first need to retrieve both load
# address and entry point for the kernel from the already
# generate uboot image before using mkimage -l.
-LINUX_APPEND_DTB += $(sep) MKIMAGE_ARGS=`$(MKIMAGE) -l $(LINUX_IMAGE_PATH) |\
+LINUX_APPEND_DTB += ; \
+ MKIMAGE_ARGS=`$(MKIMAGE) -l $(LINUX_IMAGE_PATH) |\
sed -n -e 's/Image Name:[ ]*\(.*\)/-n \1/p' -e 's/Load Address:/-a/p' -e 's/Entry Point:/-e/p'`; \
- $(MKIMAGE) -A $(MKIMAGE_ARCH) -O linux \
- -T kernel -C none $${MKIMAGE_ARGS} \
- -d $(KERNEL_ARCH_PATH)/boot/zImage $(LINUX_IMAGE_PATH);
+ for dtb in $(KERNEL_DTS_NAME); do \
+ $(MKIMAGE) -A $(MKIMAGE_ARCH) -O linux \
+ -T kernel -C none $${MKIMAGE_ARGS} \
+ -d $(KERNEL_ARCH_PATH)/boot/zImage.$${dtb} $(LINUX_IMAGE_PATH).$${dtb}; \
+ done
endif
endif
@@ -279,9 +286,20 @@ define LINUX_BUILD_CMDS
$(LINUX_APPEND_DTB)
endef
+ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB),y)
+# When a DTB was appended, install the potential several images with
+# appended DTBs.
+define LINUX_INSTALL_IMAGE
+ mkdir -p $(1)
+ cp $(KERNEL_ARCH_PATH)/boot/$(LINUX_IMAGE_NAME).* $(1)
+endef
+else
+# Otherwise, just install the unique image generated by the kernel
+# build process.
define LINUX_INSTALL_IMAGE
$(INSTALL) -m 0644 -D $(LINUX_IMAGE_PATH) $(1)/$(LINUX_IMAGE_NAME)
endef
+endif
ifeq ($(BR2_LINUX_KERNEL_INSTALL_TARGET),y)
define LINUX_INSTALL_KERNEL_IMAGE_TO_TARGET
@@ -290,7 +308,6 @@ define LINUX_INSTALL_KERNEL_IMAGE_TO_TARGET
endef
endif
-
define LINUX_INSTALL_HOST_TOOLS
# Installing dtc (device tree compiler) as host tool, if selected
if grep -q "CONFIG_DTC=y" $(@D)/.config; then \
@@ -377,13 +394,6 @@ $(error No kernel device tree source specified, check your \
BR2_LINUX_KERNEL_USE_INTREE_DTS / BR2_LINUX_KERNEL_USE_CUSTOM_DTS settings)
endif
-ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB),y)
-ifneq ($(words $(KERNEL_DTS_NAME)),1)
-$(error Kernel with appended device tree needs exactly one DTS source. \
- Check BR2_LINUX_KERNEL_INTREE_DTS_NAME or BR2_LINUX_KERNEL_CUSTOM_DTS_PATH.)
-endif
-endif
-
endif # BR_BUILDING
$(eval $(kconfig-package))
--
2.6.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 3/3] linux: don't build appended DTB image in place and support multiple images
2015-10-18 21:05 ` [Buildroot] [PATCH 3/3] linux: don't build appended DTB image in place and support multiple images Thomas Petazzoni
@ 2015-10-20 5:22 ` Peter Korsgaard
0 siblings, 0 replies; 6+ messages in thread
From: Peter Korsgaard @ 2015-10-20 5:22 UTC (permalink / raw)
To: buildroot
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
Hi,
> Currently, the linux.mk logic for appended DTB image does the
> appending of the DTB in place, directly at the end of the zImage using
> a >> sign. This is incorrect because if you run "make linux-rebuild"
> multiple times, you get the DTB appended over and over again to the
> image.
> Since keeping the 'zImage' or 'uImage' name for the appended DTB image
> is not very clear, this commit moves to using the 'zImage.<dtb>' and
> 'uImage.<dtb>' format. This way, we can clearly distinguish the
> original image from the appended one.
> In addition, this naming scheme easily allows to generate *multiple*
> appended DTB images: from one zImage, you can generate multiple
> zImage.<dtb> for several DTBs, and then generate (if requested) the
> corresponding uImage.<dtb>.
> To achieve this, this commit:
> - Changes the definition of LINUX_APPENDED_DTB to iterate over
> $(KERNEL_DTS_NAME), and generate a zImage.<dtb> image for each of
> them.
> - Changes the addition of LINUX_APPENDED_DTB for appended uImage to
> also iterate over $(KERNEL_DTS_NAME).
> - Provide a different implementation of LINUX_INSTALL_IMAGE which
> installs all the appended DTB images (but not the bare image)
> - Remove the checks that verified that only one DT name is passed
> when appended DTB is used, since we now support generating multiple
> DT images.
> Some of the tested configuration:
Thanks for the testing!
> - Appended uImage with several DTBs:
> BR2_LINUX_KERNEL_DEFCONFIG="mvebu_v7"
> BR2_LINUX_KERNEL_APPENDED_UIMAGE=y
> BR2_LINUX_KERNEL_UIMAGE_LOADADDR="0x200000"
> BR2_LINUX_KERNEL_INTREE_DTS_NAME="armada-xp-matrix armada-xp-gp armada-370-mirabox"
> Contents of output/images:
> uImage.armada-370-mirabox uImage.armada-xp-gp uImage.armada-xp-matrix
Don't we have readme's and u-boot boot scripts expecting uImage instead
of uImage.<dtb>? I guess if nothing else we need to update the readme's
to tell people to rename the file when copying to the sd card or
whatever.
Other than that the series look good to me!
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 0/3] linux: Fix appended DTB image generation
2015-10-18 21:04 [Buildroot] [PATCH 0/3] linux: Fix appended DTB image generation Thomas Petazzoni
` (2 preceding siblings ...)
2015-10-18 21:05 ` [Buildroot] [PATCH 3/3] linux: don't build appended DTB image in place and support multiple images Thomas Petazzoni
@ 2015-12-20 14:26 ` Thomas Petazzoni
3 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2015-12-20 14:26 UTC (permalink / raw)
To: buildroot
Hello,
On Sun, 18 Oct 2015 23:04:59 +0200, Thomas Petazzoni wrote:
> Thomas Petazzoni (3):
> linux: de-duplicate DTB and Linux image installation
> linux: only install the DTBs when not in appended DTB mode
> linux: don't build appended DTB image in place and support multiple
> images
Series applied. I didn't get much review, except Peter saying "Other
than that the series look good to me!", and the series has been around
for 2+ months, so I applied.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-12-20 14:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-18 21:04 [Buildroot] [PATCH 0/3] linux: Fix appended DTB image generation Thomas Petazzoni
2015-10-18 21:05 ` [Buildroot] [PATCH 1/3] linux: de-duplicate DTB and Linux image installation Thomas Petazzoni
2015-10-18 21:05 ` [Buildroot] [PATCH 2/3] linux: only install the DTBs when not in appended DTB mode Thomas Petazzoni
2015-10-18 21:05 ` [Buildroot] [PATCH 3/3] linux: don't build appended DTB image in place and support multiple images Thomas Petazzoni
2015-10-20 5:22 ` Peter Korsgaard
2015-12-20 14:26 ` [Buildroot] [PATCH 0/3] linux: Fix appended DTB image generation Thomas Petazzoni
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.