| Message ID | 1385942188-21831-1-git-send-email-jason@lakedaemon.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/01/2013 04:56 PM, Jason Cooper wrote: > Unlike other build products in the Linux kernel, there is no 'make > *install' mechanism to put devicetree blobs in a standard place. > > This patch is an attempt to fix this problem. Akin to 'make install', > this creates a new make target, dtbs_install. The script that gets > called defers to a distribution or user supplied installdtbs binary, > if found in the system. Otherwise, the default action is to install the > built dtbs into > > /lib/devicetrees/${kernel_version}/${dts_filename}.dtb > > This is done to keep dtbs from different kernel versions separate until > things have settled down. Once the dtbs are stable, and not so strongly > linked to the kernel version, the devicetree files will most likely move > to their own repo. Users will need to upgrade install scripts at that > time. > > /lib has been selected over /boot since /boot is often a FAT filesystem > and a majority of the dts filenames are longer than 8+3. > > Signed-off-by: Jason Cooper <jason@lakedaemon.net> > --- > Note: Stephen, I haven't added your Ack because I wanted to make sure you were > Ok with the change to /lib in light of /boot partitions formatted to FAT This script shouldn't be installing directly to the filesystem, but rather some staging area that then gets copied to the desired location by distro-/use-specific scripts. As such, the filesystem in /boot isn't relevant. But that said, I won't try and NAK it because of that.
On Tue, Dec 03, 2013 at 10:37:28AM -0700, Stephen Warren wrote: > On 12/01/2013 04:56 PM, Jason Cooper wrote: > > Unlike other build products in the Linux kernel, there is no 'make > > *install' mechanism to put devicetree blobs in a standard place. > > > > This patch is an attempt to fix this problem. Akin to 'make install', > > this creates a new make target, dtbs_install. The script that gets > > called defers to a distribution or user supplied installdtbs binary, > > if found in the system. Otherwise, the default action is to install the > > built dtbs into > > > > /lib/devicetrees/${kernel_version}/${dts_filename}.dtb > > > > This is done to keep dtbs from different kernel versions separate until > > things have settled down. Once the dtbs are stable, and not so strongly > > linked to the kernel version, the devicetree files will most likely move > > to their own repo. Users will need to upgrade install scripts at that > > time. > > > > /lib has been selected over /boot since /boot is often a FAT filesystem > > and a majority of the dts filenames are longer than 8+3. > > > > Signed-off-by: Jason Cooper <jason@lakedaemon.net> > > --- > > Note: Stephen, I haven't added your Ack because I wanted to make sure you were > > Ok with the change to /lib in light of /boot partitions formatted to FAT > > This script shouldn't be installing directly to the filesystem, but > rather some staging area that then gets copied to the desired location > by distro-/use-specific scripts. As such, the filesystem in /boot isn't > relevant. Why do you think that? All the other install targets install direct to the common destinations unless you ask for an alternative location. Why should this be any different?
Grant, On Sun, Dec 01, 2013 at 11:56:28PM +0000, Jason Cooper wrote: > Unlike other build products in the Linux kernel, there is no 'make > *install' mechanism to put devicetree blobs in a standard place. > > This patch is an attempt to fix this problem. Akin to 'make install', > this creates a new make target, dtbs_install. The script that gets > called defers to a distribution or user supplied installdtbs binary, > if found in the system. Otherwise, the default action is to install the > built dtbs into > > /lib/devicetrees/${kernel_version}/${dts_filename}.dtb > > This is done to keep dtbs from different kernel versions separate until > things have settled down. Once the dtbs are stable, and not so strongly > linked to the kernel version, the devicetree files will most likely move > to their own repo. Users will need to upgrade install scripts at that > time. > > /lib has been selected over /boot since /boot is often a FAT filesystem > and a majority of the dts filenames are longer than 8+3. > > Signed-off-by: Jason Cooper <jason@lakedaemon.net> > --- Is this good to go? thx, Jason. > Note: Stephen, I haven't added your Ack because I wanted to make sure you were > Ok with the change to /lib in light of /boot partitions formatted to FAT > > changes since v5: > - move make target back to arch/arm/Makefile (gcl) > - change default install location to /lib from /boot (FAT) (ojn,jac) > > changes since v4: > - move make target from arch/arm/Makefile to Makefile (swarren) > - change default install location to /boot/devicetrees/$KERNVER (swarren/gcl) > - add INSTALL_DTBS_PATH for changing build root (jac) > > changes since v3: > - drop renaming files to ${compat}.dtb (rmk/swarren) > - move installdtbs.sh to ./scripts/ (gcl) > > changes since v2: > - use fdtget instead of a modified dtc to get the board compat string > > changes since v1: > - added this patch > > Makefile | 12 +++++++++++- > arch/arm/Makefile | 10 ++++++++++ > scripts/installdtbs.sh | 33 +++++++++++++++++++++++++++++++++ > 3 files changed, 54 insertions(+), 1 deletion(-) > create mode 100644 scripts/installdtbs.sh > > diff --git a/Makefile b/Makefile > index c0c2d58e3998..240e0d52b126 100644 > --- a/Makefile > +++ b/Makefile > @@ -339,6 +339,7 @@ OBJDUMP = $(CROSS_COMPILE)objdump > AWK = awk > GENKSYMS = scripts/genksyms/genksyms > INSTALLKERNEL := installkernel > +INSTALLDTBS := installdtbs > DEPMOD = /sbin/depmod > PERL = perl > CHECK = sparse > @@ -391,7 +392,7 @@ KERNELVERSION = $(VERSION)$(if $(PATCHLEVEL),.$(PATCHLEVEL)$(if $(SUBLEVEL),.$(S > export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION > export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC > export CPP AR NM STRIP OBJCOPY OBJDUMP > -export MAKE AWK GENKSYMS INSTALLKERNEL PERL UTS_MACHINE > +export MAKE AWK GENKSYMS INSTALLKERNEL INSTALLDTBS PERL UTS_MACHINE > export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS > > export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS > @@ -704,6 +705,15 @@ export KBUILD_IMAGE ?= vmlinux > export INSTALL_PATH ?= /boot > > # > +# INSTALL_DTBS_PATH specifies a prefix for relocations required by build roots. > +# Like INSTALL_MOD_PATH, it isn't defined in the Makefile, but can be passed as > +# an argument if needed. > +# > + > +DTBBOOT = $(INSTALL_DTBS_PATH)/lib/devicetrees/$(KERNELRELEASE) > +export DTBBOOT > + > +# > # INSTALL_MOD_PATH specifies a prefix to MODLIB for module directory > # relocations required by build roots. This is not defined in the > # makefile but the argument can be passed to make if needed. > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > index c99b1086d83d..400a3a7ee899 100644 > --- a/arch/arm/Makefile > +++ b/arch/arm/Makefile > @@ -314,6 +314,12 @@ PHONY += dtbs > dtbs: scripts > $(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) dtbs > > +PHONY += dtbs_install > + > +dtbs_install: dtbs > + $(CONFIG_SHELL) $(srctree)/scripts/installdtbs.sh $(KERNELRELEASE) \ > + "$(DTBBOOT)" "$(srctree)" > + > # We use MRPROPER_FILES and CLEAN_FILES now > archclean: > $(Q)$(MAKE) $(clean)=$(boot) > @@ -331,6 +337,10 @@ define archhelp > echo ' bootpImage - Combined zImage and initial RAM disk' > echo ' (supply initrd image via make variable INITRD=<path>)' > echo '* dtbs - Build device tree blobs for enabled boards' > + echo ' dtbs_install - Install dtbs' > + echo ' Install using (your) ~/bin/$(INSTALLDTBS) or' > + echo ' (distribution) /sbin/$(INSTALLDTBS) or' > + echo ' install to $(DTBBOOT)' > echo ' install - Install uncompressed kernel' > echo ' zinstall - Install compressed kernel' > echo ' uinstall - Install U-Boot wrapped compressed kernel' > diff --git a/scripts/installdtbs.sh b/scripts/installdtbs.sh > new file mode 100644 > index 000000000000..11027f00c3a4 > --- /dev/null > +++ b/scripts/installdtbs.sh > @@ -0,0 +1,33 @@ > +#!/bin/sh > +# > +# This file is subject to the terms and conditions of the GNU General Public > +# License. See the file "COPYING" in the main directory of this archive > +# for more details. > +# > +# Copyright (C) 1995 by Linus Torvalds > +# > +# Adapted from code in arch/i386/boot/Makefile by H. Peter Anvin > +# > +# Further adapted from arch/x86/boot/install.sh by Jason Cooper > +# > +# "make dtbs_install" script > +# > +# Arguments: > +# $1 - kernel version > +# $2 - default install path (blank if root directory) > +# $3 - directory containing dtbs > +# > + > +# User may have a custom install script > + > +if [ -x ~/bin/${INSTALLDTBS} ]; then exec ~/bin/${INSTALLDTBS} "$@"; fi > +if [ -x /sbin/${INSTALLDTBS} ]; then exec /sbin/${INSTALLDTBS} "$@"; fi > + > +# Default install > +[ -d "$2" ] && rm -rf "$2" > + > +mkdir -p "$2" > + > +for dtb in `find "$3" -name "*.dtb"`; do > + cp "$dtb" "$2/${dtb##*/}" > +done > -- > 1.8.4.4 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 10 Jan 2014 13:29:23 -0500, Jason Cooper <jason@lakedaemon.net> wrote: > Grant, > > On Sun, Dec 01, 2013 at 11:56:28PM +0000, Jason Cooper wrote: > > Unlike other build products in the Linux kernel, there is no 'make > > *install' mechanism to put devicetree blobs in a standard place. > > > > This patch is an attempt to fix this problem. Akin to 'make install', > > this creates a new make target, dtbs_install. The script that gets > > called defers to a distribution or user supplied installdtbs binary, > > if found in the system. Otherwise, the default action is to install the > > built dtbs into > > > > /lib/devicetrees/${kernel_version}/${dts_filename}.dtb > > > > This is done to keep dtbs from different kernel versions separate until > > things have settled down. Once the dtbs are stable, and not so strongly > > linked to the kernel version, the devicetree files will most likely move > > to their own repo. Users will need to upgrade install scripts at that > > time. > > > > /lib has been selected over /boot since /boot is often a FAT filesystem > > and a majority of the dts filenames are longer than 8+3. > > > > Signed-off-by: Jason Cooper <jason@lakedaemon.net> > > --- > > Is this good to go? I took another look at it and did some rework. It bothered my that the install script didnt' have any knowledge of the actual build targets and just blindly copied the files it finds. I've reworked the patch to make each file a separate target and also fixed a few bugs in the process. I also removed the call out to an external script. I'm not convinced a call out hook is the best approach when a buildroot tool can post process the install directory. It wouldn't be hard to add back in, but I don't want it to block this feature. I'll post my version in a minute. g.
Okay, I'm digging up an old version of this patch - v7 was merged but I find *nowhere* where that version was posted to people involved in this discussion. The reason is that I would've commented on v7, because of this stupid thing (which is now in scripts/Makefile.dtbsinst): + $(Q)if [ -d $(INSTALL_DTBS_PATH).old ]; then rm -rf $(INSTALL_DTBS_PATH).old; fi + $(Q)if [ -d $(INSTALL_DTBS_PATH) ]; then mv $(INSTALL_DTBS_PATH) $(INSTALL_DTBS_PATH).old; fi + $(Q)mkdir -p $(INSTALL_DTBS_PATH) What gives any kernel installation target the right to move a directory out of the way? Let's say that you do this: make install INSTALL_PATH=$sys_root/boot make dtbs_install INSTALL_DTBS_PATH=$sys_root/boot The result is that dtbs_install thinks it has the right to rename that boot directory to boot.old, create a new one, and place the DTBs in there, thereby leaving you with no kernel to boot - and if you run it again, it _deletes_ the original directory. No *other* kernel install target has this behaviour, not even 'make modules_installl'. This is stupid and dangerous behaviour. At the very least, this behaviour should be *well* documented. Linus, will you take a patch to remove the lines moving a pre-existing INSTALL_DTBS_PATH directory out of the way?
Russell, On Sat, Mar 28, 2015 at 01:23:20PM +0000, Russell King - ARM Linux wrote: > Okay, I'm digging up an old version of this patch - v7 was merged but > I find *nowhere* where that version was posted to people involved in > this discussion. fwiw, I just went through my archive of patch submissions: $ ls ~/projects/linux/dtbs_install/ v2 v3 v4 v5 v6 $ I never wrote a v7. :-( Regardless, > The reason is that I would've commented on v7, because of this stupid > thing (which is now in scripts/Makefile.dtbsinst): > > + $(Q)if [ -d $(INSTALL_DTBS_PATH).old ]; then rm -rf $(INSTALL_DTBS_PATH).old; fi > + $(Q)if [ -d $(INSTALL_DTBS_PATH) ]; then mv $(INSTALL_DTBS_PATH) $(INSTALL_DTBS_PATH).old; fi > + $(Q)mkdir -p $(INSTALL_DTBS_PATH) > > What gives any kernel installation target the right to move a directory > out of the way? > > Let's say that you do this: > > make install INSTALL_PATH=$sys_root/boot > make dtbs_install INSTALL_DTBS_PATH=$sys_root/boot > > The result is that dtbs_install thinks it has the right to rename that > boot directory to boot.old, create a new one, and place the DTBs in > there, thereby leaving you with no kernel to boot - and if you run it > again, it _deletes_ the original directory. > > No *other* kernel install target has this behaviour, not even 'make > modules_installl'. This is stupid and dangerous behaviour. At the > very least, this behaviour should be *well* documented. you are correct. This should be fixed. thx, Jason.
On Sat, Mar 28, 2015 at 8:58 AM, Jason Cooper <jason@lakedaemon.net> wrote: > Russell, > > On Sat, Mar 28, 2015 at 01:23:20PM +0000, Russell King - ARM Linux wrote: >> Okay, I'm digging up an old version of this patch - v7 was merged but >> I find *nowhere* where that version was posted to people involved in >> this discussion. > > fwiw, I just went through my archive of patch submissions: > > $ ls ~/projects/linux/dtbs_install/ > v2 v3 v4 v5 v6 > $ > > I never wrote a v7. :-( Regardless, It was posted by Grant, and posted to LKML, devicetree@ and linux-kbuild. The patch was directly cc:d to Jason, Michal Marek, Russell and Rob Herring. -Olof
On Sun, Mar 29, 2015 at 01:34:41PM -0700, Olof Johansson wrote: > On Sat, Mar 28, 2015 at 8:58 AM, Jason Cooper <jason@lakedaemon.net> wrote: > > Russell, > > > > On Sat, Mar 28, 2015 at 01:23:20PM +0000, Russell King - ARM Linux wrote: > >> Okay, I'm digging up an old version of this patch - v7 was merged but > >> I find *nowhere* where that version was posted to people involved in > >> this discussion. > > > > fwiw, I just went through my archive of patch submissions: > > > > $ ls ~/projects/linux/dtbs_install/ > > v2 v3 v4 v5 v6 > > $ > > > > I never wrote a v7. :-( Regardless, > > It was posted by Grant, and posted to LKML, devicetree@ and > linux-kbuild. The patch was directly cc:d to Jason, Michal Marek, > Russell and Rob Herring. I couldn't find a copy of it - grepping for "^Subject:.*dtb.*install" around December 2013 and before only found Jason's original patches. The commit introducing it says: commit f4d4ffc03efc864645b990e1d579bbe1b8e358a4 Author: Jason Cooper <jason@lakedaemon.net> AuthorDate: Sun Dec 1 23:56:28 2013 +0000 Commit: Grant Likely <grant.likely@linaro.org> CommitDate: Thu Feb 20 15:53:39 2014 +0000 kbuild: dtbs_install: new make target So it seems that I was looking at the wrong date range... Grant did send the patch on the 4th February, which I never got around to reading.
On Sun, Mar 29, 2015 at 01:34:41PM -0700, Olof Johansson wrote: > On Sat, Mar 28, 2015 at 8:58 AM, Jason Cooper <jason@lakedaemon.net> wrote: > > Russell, > > > > On Sat, Mar 28, 2015 at 01:23:20PM +0000, Russell King - ARM Linux wrote: > >> Okay, I'm digging up an old version of this patch - v7 was merged but > >> I find *nowhere* where that version was posted to people involved in > >> this discussion. > > > > fwiw, I just went through my archive of patch submissions: > > > > $ ls ~/projects/linux/dtbs_install/ > > v2 v3 v4 v5 v6 > > $ > > > > I never wrote a v7. :-( Regardless, > > It was posted by Grant, and posted to LKML, devicetree@ and > linux-kbuild. The patch was directly cc:d to Jason, Michal Marek, > Russell and Rob Herring. Ah, now I found it: http://lkml.iu.edu/hypermail/linux/kernel/1402.0/01294.html My initial search missed it because it didn't have v7 in the subject. I should just trust the Google, and not my grep-fu. :-) Thanks for the clarification. thx, Jason.
diff --git a/Makefile b/Makefile index c0c2d58e3998..240e0d52b126 100644 --- a/Makefile +++ b/Makefile @@ -339,6 +339,7 @@ OBJDUMP = $(CROSS_COMPILE)objdump AWK = awk GENKSYMS = scripts/genksyms/genksyms INSTALLKERNEL := installkernel +INSTALLDTBS := installdtbs DEPMOD = /sbin/depmod PERL = perl CHECK = sparse @@ -391,7 +392,7 @@ KERNELVERSION = $(VERSION)$(if $(PATCHLEVEL),.$(PATCHLEVEL)$(if $(SUBLEVEL),.$(S export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC export CPP AR NM STRIP OBJCOPY OBJDUMP -export MAKE AWK GENKSYMS INSTALLKERNEL PERL UTS_MACHINE +export MAKE AWK GENKSYMS INSTALLKERNEL INSTALLDTBS PERL UTS_MACHINE export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS @@ -704,6 +705,15 @@ export KBUILD_IMAGE ?= vmlinux export INSTALL_PATH ?= /boot # +# INSTALL_DTBS_PATH specifies a prefix for relocations required by build roots. +# Like INSTALL_MOD_PATH, it isn't defined in the Makefile, but can be passed as +# an argument if needed. +# + +DTBBOOT = $(INSTALL_DTBS_PATH)/lib/devicetrees/$(KERNELRELEASE) +export DTBBOOT + +# # INSTALL_MOD_PATH specifies a prefix to MODLIB for module directory # relocations required by build roots. This is not defined in the # makefile but the argument can be passed to make if needed. diff --git a/arch/arm/Makefile b/arch/arm/Makefile index c99b1086d83d..400a3a7ee899 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -314,6 +314,12 @@ PHONY += dtbs dtbs: scripts $(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) dtbs +PHONY += dtbs_install + +dtbs_install: dtbs + $(CONFIG_SHELL) $(srctree)/scripts/installdtbs.sh $(KERNELRELEASE) \ + "$(DTBBOOT)" "$(srctree)" + # We use MRPROPER_FILES and CLEAN_FILES now archclean: $(Q)$(MAKE) $(clean)=$(boot) @@ -331,6 +337,10 @@ define archhelp echo ' bootpImage - Combined zImage and initial RAM disk' echo ' (supply initrd image via make variable INITRD=<path>)' echo '* dtbs - Build device tree blobs for enabled boards' + echo ' dtbs_install - Install dtbs' + echo ' Install using (your) ~/bin/$(INSTALLDTBS) or' + echo ' (distribution) /sbin/$(INSTALLDTBS) or' + echo ' install to $(DTBBOOT)' echo ' install - Install uncompressed kernel' echo ' zinstall - Install compressed kernel' echo ' uinstall - Install U-Boot wrapped compressed kernel' diff --git a/scripts/installdtbs.sh b/scripts/installdtbs.sh new file mode 100644 index 000000000000..11027f00c3a4 --- /dev/null +++ b/scripts/installdtbs.sh @@ -0,0 +1,33 @@ +#!/bin/sh +# +# This file is subject to the terms and conditions of the GNU General Public +# License. See the file "COPYING" in the main directory of this archive +# for more details. +# +# Copyright (C) 1995 by Linus Torvalds +# +# Adapted from code in arch/i386/boot/Makefile by H. Peter Anvin +# +# Further adapted from arch/x86/boot/install.sh by Jason Cooper +# +# "make dtbs_install" script +# +# Arguments: +# $1 - kernel version +# $2 - default install path (blank if root directory) +# $3 - directory containing dtbs +# + +# User may have a custom install script + +if [ -x ~/bin/${INSTALLDTBS} ]; then exec ~/bin/${INSTALLDTBS} "$@"; fi +if [ -x /sbin/${INSTALLDTBS} ]; then exec /sbin/${INSTALLDTBS} "$@"; fi + +# Default install +[ -d "$2" ] && rm -rf "$2" + +mkdir -p "$2" + +for dtb in `find "$3" -name "*.dtb"`; do + cp "$dtb" "$2/${dtb##*/}" +done
Unlike other build products in the Linux kernel, there is no 'make *install' mechanism to put devicetree blobs in a standard place. This patch is an attempt to fix this problem. Akin to 'make install', this creates a new make target, dtbs_install. The script that gets called defers to a distribution or user supplied installdtbs binary, if found in the system. Otherwise, the default action is to install the built dtbs into /lib/devicetrees/${kernel_version}/${dts_filename}.dtb This is done to keep dtbs from different kernel versions separate until things have settled down. Once the dtbs are stable, and not so strongly linked to the kernel version, the devicetree files will most likely move to their own repo. Users will need to upgrade install scripts at that time. /lib has been selected over /boot since /boot is often a FAT filesystem and a majority of the dts filenames are longer than 8+3. Signed-off-by: Jason Cooper <jason@lakedaemon.net> --- Note: Stephen, I haven't added your Ack because I wanted to make sure you were Ok with the change to /lib in light of /boot partitions formatted to FAT changes since v5: - move make target back to arch/arm/Makefile (gcl) - change default install location to /lib from /boot (FAT) (ojn,jac) changes since v4: - move make target from arch/arm/Makefile to Makefile (swarren) - change default install location to /boot/devicetrees/$KERNVER (swarren/gcl) - add INSTALL_DTBS_PATH for changing build root (jac) changes since v3: - drop renaming files to ${compat}.dtb (rmk/swarren) - move installdtbs.sh to ./scripts/ (gcl) changes since v2: - use fdtget instead of a modified dtc to get the board compat string changes since v1: - added this patch Makefile | 12 +++++++++++- arch/arm/Makefile | 10 ++++++++++ scripts/installdtbs.sh | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 scripts/installdtbs.sh