deb-pkg: Add device tree blobs to the package

PATCH Superseded Arnaud Patard (Rtp) 4 participants 12 messages Jan 14, 2015 – Jan 23, 2015 Kbuild

lore

Share & tools

lore

Arnaud Patard (Rtp)

author

2015-01-14 12:48 UTC #0 lore raw

v2 cover deb-pkg: Add device tree blobs to the package

Hi,

Looks like I've never sent the v2 of this patch, so let's try to correct this.

Thanks,
Arnaud

Arnaud Patard (Rtp)

author

2015-01-14 12:48 UTC #1 lore raw

PATCH v2 1/1 deb-pkg: Add device tree blobs to the package

When building a package with make deb-pkg (say, for arm), the dtb files are not added to the package. Given that things are still evolving on arm, it make sense to have them along with the kernel and modules.

v2: make use of dtbs_install

Signed-off-by Arnaud Patard <[email protected]>

---

scripts/package/builddeb +4 -0

Index: linux-next/scripts/package/builddeb
===================================================================
@@ -143,6 +143,10 @@ else
 	cp arch/$ARCH/boot/$KBUILD_IMAGE "$tmpdir/$installed_image_path"
 fi
 
+if grep -q "^CONFIG_OF=y" .config ; then
+	make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
+fi
+
 if grep -q '^CONFIG_MODULES=y' $KCONFIG_CONFIG ; then
 	INSTALL_MOD_PATH="$tmpdir" $MAKE KBUILD_SRC= modules_install
 	rm -f "$tmpdir/lib/modules/$version/build"

Ben Hutchings

2015-01-14 12:51 UTC #2 lore raw

[Please submit patches inline.]

Quoted 14 lines · Arnaud Patard

When building a package with make deb-pkg (say, for arm), the dtb files are not added to the package. Given that things are still evolving on arm, it make sense to have them along with the kernel and modules.
v2: make use of dtbs_install
Signed-off-by Arnaud Patard <[email protected]>
---
Index: linux-next/scripts/package/builddeb
===================================================================

scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 else · line 148

Quoted 6 lines

scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 renamed from scripts/package/builddeb 2015-01-14 13:04:45.845922441 +0100 +3 -0
@@ -143,6 +143,10 @@ else
        cp arch/$ARCH/boot/$KBUILD_IMAGE "$tmpdir/$installed_image_path"
 fi
 
+if grep -q "^CONFIG_OF=y" .config ; then
+       make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
+fi

Only arm and arm64 support that target. You should maybe run something like 'make -n dtbs_install >/dev/null 2>&1' first to check that the target is defined.

Ben.

Quoted 4 lines

scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 renamed from scripts/package/builddeb 2015-01-14 13:04:45.845922441 +0100 +0 -0
 if grep -q '^CONFIG_MODULES=y' $KCONFIG_CONFIG ; then
        INSTALL_MOD_PATH="$tmpdir" $MAKE KBUILD_SRC= modules_install
        rm -f "$tmpdir/lib/modules/$version/build"

Signature · 2 lines

Ben Hutchings
The world is coming to an end.	Please log off.

Arnaud Patard (Rtp)

author

2015-01-19 23:14 UTC #3 lore raw

Quoted 1 line · Ben Hutchings

[Please submit patches inline.]

I've no way to control the way quilt is sending mails.

Quoted 19 lines · Arnaud Patard

Quoted 17 lines
When building a package with make deb-pkg (say, for arm), the dtb files are not added to the package. Given that things are still evolving on arm, it make sense to have them along with the kernel and modules.
v2: make use of dtbs_install
Signed-off-by Arnaud Patard <[email protected]>
---
Index: linux-next/scripts/package/builddeb
===================================================================
scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 renamed from scripts/package/builddeb 2015-01-14 13:04:45.845922441 +0100 +0 -0
@@ -143,6 +143,10 @@ else
        cp arch/$ARCH/boot/$KBUILD_IMAGE "$tmpdir/$installed_image_path"
 fi
 

Quoted 7 lines

scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 else · line 148
Quoted 3 lines
scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 renamed from scripts/package/builddeb 2015-01-14 13:04:45.845922441 +0100 +3 -0
+if grep -q "^CONFIG_OF=y" .config ; then
+       make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
+fi
Only arm and arm64 support that target. You should maybe run something like 'make -n dtbs_install >/dev/null 2>&1' first to check that the target is defined.

There's a 'set -e' on top of the script so using make -n will likely result in the script failing, which wouldn't be so nice imho.

From a quick untested guess, I see 2 way of solving this:

make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install || /bin/true

set +e
make -n dtbs_install >/dev/null 2>&1
ret=$?
set -e
if [ $ret -eq 0 ]; then
make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
fi

Any preference ?

Arnaud

Ben Hutchings

2015-01-20 16:05 UTC #4 lore raw

Quoted 33 lines · Arnaud Patard

Quoted 1 line · Ben Hutchings
[Please submit patches inline.]
I've no way to control the way quilt is sending mails.
Quoted 19 lines · Arnaud Patard
Quoted 17 lines
When building a package with make deb-pkg (say, for arm), the dtb files are not added to the package. Given that things are still evolving on arm, it make sense to have them along with the kernel and modules.
v2: make use of dtbs_install
Signed-off-by Arnaud Patard <[email protected]>
---
Index: linux-next/scripts/package/builddeb
===================================================================
scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 renamed from scripts/package/builddeb 2015-01-14 13:04:45.845922441 +0100 +0 -0
@@ -143,6 +143,10 @@ else
        cp arch/$ARCH/boot/$KBUILD_IMAGE "$tmpdir/$installed_image_path"
 fi
 
Quoted 7 lines
scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 else · line 148
Quoted 3 lines
scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 renamed from scripts/package/builddeb 2015-01-14 13:04:45.845922441 +0100 +3 -0
+if grep -q "^CONFIG_OF=y" .config ; then
+       make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
+fi
Only arm and arm64 support that target. You should maybe run something like 'make -n dtbs_install >/dev/null 2>&1' first to check that the target is defined.

Quoted 2 lines

There's a 'set -e' on top of the script so using make -n will likely result in the script failing, which wouldn't be so nice imho.

[...]

That's why you use it with the if statement:

# Only some architectures with OF support have this target
if make -n dtbs_install >/dev/null 2>&1; then
make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
fi

Ben.

Signature · 2 lines

Ben Hutchings
Larkinson's Law: All laws are basically false.

Fathi Boudra

2015-01-22 10:24 UTC #5 lore raw

Hi,

Quoted 36 lines · Ben Hutchings

Quoted 33 lines · Arnaud Patard
Quoted 1 line · Ben Hutchings
[Please submit patches inline.]
I've no way to control the way quilt is sending mails.
Quoted 19 lines · Arnaud Patard
Quoted 17 lines
When building a package with make deb-pkg (say, for arm), the dtb files are not added to the package. Given that things are still evolving on arm, it make sense to have them along with the kernel and modules.
v2: make use of dtbs_install
Signed-off-by Arnaud Patard <[email protected]>
---
Index: linux-next/scripts/package/builddeb
===================================================================
scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 renamed from scripts/package/builddeb 2015-01-14 13:04:45.845922441 +0100 +0 -0
@@ -143,6 +143,10 @@ else
        cp arch/$ARCH/boot/$KBUILD_IMAGE "$tmpdir/$installed_image_path"
 fi
 
Quoted 7 lines
scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 else · line 148
Quoted 3 lines
scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 renamed from scripts/package/builddeb 2015-01-14 13:04:45.845922441 +0100 +3 -0
+if grep -q "^CONFIG_OF=y" .config ; then
+       make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
+fi
Only arm and arm64 support that target. You should maybe run something like 'make -n dtbs_install >/dev/null 2>&1' first to check that the target is defined.
Quoted 2 lines
There's a 'set -e' on top of the script so using make -n will likely result in the script failing, which wouldn't be so nice imho.

Quoted 8 lines

[...]
That's why you use it with the if statement:
# Only some architectures with OF support have this target
if make -n dtbs_install >/dev/null 2>&1; then
make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
fi

Would it be acceptable to have a fallback when dtbs_install isn't available? It will allow to cover e.g. LTS 3.10 kernel which don't have the target

for example (i.e. can't be upstreamed as-is), I'm using this hack locally:
https://git.linaro.org/kernel/linux-linaro-tracking.git/blob/HEAD:/scripts/package/builddeb#l334

Quoted 1 line

Ben.

Cheers,
Fathi

Ben Hutchings

2015-01-22 13:15 UTC #6 lore raw

Quoted 48 lines · Fathi Boudra

Hi,
Quoted 36 lines · Ben Hutchings
Quoted 33 lines · Arnaud Patard
Quoted 1 line · Ben Hutchings
[Please submit patches inline.]
I've no way to control the way quilt is sending mails.
Quoted 19 lines · Arnaud Patard
Quoted 17 lines
When building a package with make deb-pkg (say, for arm), the dtb files are not added to the package. Given that things are still evolving on arm, it make sense to have them along with the kernel and modules.
v2: make use of dtbs_install
Signed-off-by Arnaud Patard <[email protected]>
---
Index: linux-next/scripts/package/builddeb
===================================================================
scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 renamed from scripts/package/builddeb 2015-01-14 13:04:45.845922441 +0100 +0 -0
@@ -143,6 +143,10 @@ else
        cp arch/$ARCH/boot/$KBUILD_IMAGE "$tmpdir/$installed_image_path"
 fi
 
Quoted 7 lines
scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 else · line 148
Quoted 3 lines
scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100 renamed from scripts/package/builddeb 2015-01-14 13:04:45.845922441 +0100 +3 -0
+if grep -q "^CONFIG_OF=y" .config ; then
+       make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
+fi
Only arm and arm64 support that target. You should maybe run something like 'make -n dtbs_install >/dev/null 2>&1' first to check that the target is defined.
Quoted 2 lines
There's a 'set -e' on top of the script so using make -n will likely result in the script failing, which wouldn't be so nice imho.
Quoted 8 lines
[...]
That's why you use it with the if statement:
# Only some architectures with OF support have this target
if make -n dtbs_install >/dev/null 2>&1; then
make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
fi

Quoted 5 lines

Would it be acceptable to have a fallback when dtbs_install isn't available? It will allow to cover e.g. LTS 3.10 kernel which don't have the target
for example (i.e. can't be upstreamed as-is), I'm using this hack locally:
https://git.linaro.org/kernel/linux-linaro-tracking.git/blob/HEAD:/scripts/package/builddeb#l334

If there are any architectures that build .dtb files but don't have a dtbs_install target, they should be fixed rather than adding a workaround here.

If you want this in some other branch then you should get the addition of the dtbs_install target backported.

Ben.

Signature · 2 lines

Ben Hutchings
I haven't lost my mind; it's backed up on tape somewhere.

Michal Marek

2015-01-22 13:40 UTC #7 lore raw

In reply to Ben Hutchings

Quoted 6 lines · Ben Hutchings

Quoted 5 lines · Arnaud Patard
Quoted 3 lines · Ben Hutchings
Only arm and arm64 support that target. You should maybe run something like 'make -n dtbs_install >/dev/null 2>&1' first to check that the target is defined.

Quoted 8 lines

Quoted 2 lines
There's a 'set -e' on top of the script so using make -n will likely result in the script failing, which wouldn't be so nice imho.
[...]
That's why you use it with the if statement:
# Only some architectures with OF support have this target if make -n dtbs_install >/dev/null 2>&1; then

The problem is that kbuild does not support make -n and some of the commands are always executed. So the command will silently build the blobs and just not install them. Better do

if grep -q dtbs_install "arch/$SRCARCH/Makefile"; then
...

and hope that there won't be any comment causing a false positive. Another option is to grep 'make help' output.

Michal

Ben Hutchings

2015-01-22 14:19 UTC #8 lore raw

Quoted 11 lines · Michal Marek

Quoted 10 lines · Ben Hutchings
Quoted 7 lines · Arnaud Patard
Quoted 3 lines · Ben Hutchings
Only arm and arm64 support that target. You should maybe run something like 'make -n dtbs_install >/dev/null 2>&1' first to check that the target is defined.
There's a 'set -e' on top of the script so using make -n will likely result in the script failing, which wouldn't be so nice imho.
[...]

Quoted 8 lines

Quoted 4 lines
That's why you use it with the if statement:
# Only some architectures with OF support have this target if make -n dtbs_install >/dev/null 2>&1; then
The problem is that kbuild does not support make -n and some of the commands are always executed. So the command will silently build the blobs and just not install them.

If the dtbs_install target is defined, they should already have been built at this point. If the target is not defined, why would it do anything?

Of course it should be $MAKE not make.

Quoted 7 lines

Better do
if grep -q dtbs_install "arch/$SRCARCH/Makefile"; then
...
and hope that there won't be any comment causing a false positive. Another option is to grep 'make help' output.

Ugh.

Ben.

Signature · 2 lines

Ben Hutchings
I haven't lost my mind; it's backed up on tape somewhere.

Michal Marek

2015-01-22 15:30 UTC #9 lore raw

Quoted 17 lines · Ben Hutchings

Quoted 16 lines · Michal Marek
Quoted 14 lines · Ben Hutchings
Quoted 7 lines · Arnaud Patard
Quoted 3 lines · Ben Hutchings
Only arm and arm64 support that target. You should maybe run something like 'make -n dtbs_install >/dev/null 2>&1' first to check that the target is defined.
There's a 'set -e' on top of the script so using make -n will likely result in the script failing, which wouldn't be so nice imho.
[...]
That's why you use it with the if statement:
# Only some architectures with OF support have this target if make -n dtbs_install >/dev/null 2>&1; then

Quoted 7 lines

Quoted 3 lines
The problem is that kbuild does not support make -n and some of the commands are always executed. So the command will silently build the blobs and just not install them.
If the dtbs_install target is defined, they should already have been built at this point. If the target is not defined, why would it do anything?

The point is that make -n does not work with kbuild. It might work for this specific case, but there is no guarantee that it will continue to do so.

Quoted 7 lines

Quoted 5 lines
if grep -q dtbs_install "arch/$SRCARCH/Makefile"; then
...
and hope that there won't be any comment causing a false positive. Another option is to grep 'make help' output.
Ugh.

I know that both options are ugly :).

Michal

Arnaud Patard (Rtp)

author

2015-01-22 15:58 UTC #10 lore raw

Quoted 22 lines · Michal Marek

Quoted 21 lines · Ben Hutchings
Quoted 19 lines · Michal Marek
Quoted 6 lines · Ben Hutchings
Quoted 5 lines · Arnaud Patard
Quoted 3 lines · Ben Hutchings
Only arm and arm64 support that target. You should maybe run something like 'make -n dtbs_install >/dev/null 2>&1' first to check that the target is defined.
Quoted 8 lines
Quoted 2 lines
There's a 'set -e' on top of the script so using make -n will likely result in the script failing, which wouldn't be so nice imho.
[...]
That's why you use it with the if statement:
# Only some architectures with OF support have this target if make -n dtbs_install >/dev/null 2>&1; then
The problem is that kbuild does not support make -n and some of the commands are always executed. So the command will silently build the blobs and just not install them.

Quoted 8 lines

Quoted 3 lines
If the dtbs_install target is defined, they should already have been built at this point. If the target is not defined, why would it do anything?
The point is that make -n does not work with kbuild. It might work for this specific case, but there is no guarantee that it will continue to do so.

Ok, so a patch with make -n will be refused, right ?

Quoted 10 lines

Quoted 8 lines
Quoted 6 lines
if grep -q dtbs_install "arch/$SRCARCH/Makefile"; then
...
and hope that there won't be any comment causing a false positive. Another option is to grep 'make help' output.
Ugh.
I know that both options are ugly :).

Let's go to your first options then. I feel not really motivated by parsing 'make help' output.

Arnaud

Michal Marek

2015-01-23 09:55 UTC #11 lore raw

Quoted 28 lines · Arnaud Patard (Rtp)

Quoted 26 lines · Michal Marek
Quoted 24 lines · Ben Hutchings
Quoted 11 lines · Michal Marek
Quoted 10 lines · Ben Hutchings
Quoted 7 lines · Arnaud Patard
Quoted 3 lines · Ben Hutchings
Only arm and arm64 support that target. You should maybe run something like 'make -n dtbs_install >/dev/null 2>&1' first to check that the target is defined.
There's a 'set -e' on top of the script so using make -n will likely result in the script failing, which wouldn't be so nice imho.
[...]
Quoted 8 lines
Quoted 4 lines
That's why you use it with the if statement:
# Only some architectures with OF support have this target if make -n dtbs_install >/dev/null 2>&1; then
The problem is that kbuild does not support make -n and some of the commands are always executed. So the command will silently build the blobs and just not install them.
If the dtbs_install target is defined, they should already have been built at this point. If the target is not defined, why would it do anything?

Quoted 6 lines

Quoted 4 lines
The point is that make -n does not work with kbuild. It might work for this specific case, but there is no guarantee that it will continue to do so.
Ok, so a patch with make -n will be refused, right ?

I'm not trying to veto such patch, I just wanted to point out that make -n is not the best idea to do with kbuild.

Quoted 2 lines

Let's go to your first options then. I feel not really motivated by parsing 'make help' output.

Whatever. You can also have a static list of archs supporting dtbs_install, like we (still) have in scripts/headers.sh. It would be guaranteed to works as designed, but obviously not future-proof.

Michal