diff mbox series

Reapply "kbuild: Create directory for target DTB"

Message ID 20231212161610.100862-1-matthias.schiffer@ew.tq-group.com (mailing list archive)
State New, archived
Headers show
Series Reapply "kbuild: Create directory for target DTB" | expand

Commit Message

Matthias Schiffer Dec. 12, 2023, 4:16 p.m. UTC
This reverts commit dd7699e37f289fa433f42c6bcc108468c8b198c0.

On powerpc, dtb-y is usually empty unless CONFIG_OF_ALL_DTBS is set. While
passing a DTB as a make target explicitly works fine, individual DTB
builds may also be pulled in as dependencies by cuImage.% and similar
targets. In this case, nothing creates the arch/powerpc/dts directory,
causing out-of-tree builds to fail.

Fixes: dd7699e37f28 ("Revert "kbuild: Create directory for target DTB"")
Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---
 scripts/Makefile.lib | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Masahiro Yamada Dec. 12, 2023, 5:12 p.m. UTC | #1
On Wed, Dec 13, 2023 at 1:17 AM Matthias Schiffer
<matthias.schiffer@ew.tq-group.com> wrote:
>
> This reverts commit dd7699e37f289fa433f42c6bcc108468c8b198c0.
>
> On powerpc, dtb-y is usually empty unless CONFIG_OF_ALL_DTBS is set. While
> passing a DTB as a make target explicitly works fine, individual DTB
> builds may also be pulled in as dependencies by cuImage.% and similar
> targets. In this case, nothing creates the arch/powerpc/dts directory,
> causing out-of-tree builds to fail.
>
> Fixes: dd7699e37f28 ("Revert "kbuild: Create directory for target DTB"")
> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> ---



NACK.

%.dtb is generated by if_changed_dep.

Each Makefile is responsible for adding %.dtb to 'targets'
if it is pulled in as dependencies of other images.

If it does not work for PowerPC, it is a bug in PowerPC Makefile.


Just checking arch/powerpc/boot/Makefile,
it adds dts/%.dtb and dts/fsl/%.dtb to 'targets'. [1] [2]

cuImage.% should be file, but it does not cover all images.

Fix arch/powerpc/boot/Makefile.



[1] https://github.com/torvalds/linux/blob/v6.7-rc5/arch/powerpc/boot/Makefile#L386
[2] https://github.com/torvalds/linux/blob/v6.7-rc5/arch/powerpc/boot/Makefile#L388



>  scripts/Makefile.lib | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 1a965fe68e011..3fe0fc46badfe 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -389,7 +389,8 @@ $(obj)/%.dtbo.S: $(obj)/%.dtbo FORCE
>         $(call if_changed,wrap_S_dtb)
>
>  quiet_cmd_dtc = DTC     $@
> -cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
> +cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
> +       $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
>         $(DTC) -o $@ -b 0 \
>                 $(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
>                 -d $(depfile).dtc.tmp $(dtc-tmp) ; \
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> https://www.tq-group.com/
>


--
Best Regards

Masahiro Yamada
Matthias Schiffer Dec. 13, 2023, 11:21 a.m. UTC | #2
On Tue, 2023-12-12 at 17:13 +0000, Masahiro Yamada wrote:
> 
> 
> On Wed, Dec 13, 2023 at 1:17 AM Matthias Schiffer
> <matthias.schiffer@ew.tq-group.com> wrote:
> > 
> > This reverts commit dd7699e37f289fa433f42c6bcc108468c8b198c0.
> > 
> > On powerpc, dtb-y is usually empty unless CONFIG_OF_ALL_DTBS is set. While
> > passing a DTB as a make target explicitly works fine, individual DTB
> > builds may also be pulled in as dependencies by cuImage.% and similar
> > targets. In this case, nothing creates the arch/powerpc/dts directory,
> > causing out-of-tree builds to fail.
> > 
> > Fixes: dd7699e37f28 ("Revert "kbuild: Create directory for target DTB"")
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > ---
> 
> 
> 
> NACK.
> 
> %.dtb is generated by if_changed_dep.
> 
> Each Makefile is responsible for adding %.dtb to 'targets'
> if it is pulled in as dependencies of other images.
> 
> If it does not work for PowerPC, it is a bug in PowerPC Makefile.
> 
> 
> Just checking arch/powerpc/boot/Makefile,
> it adds dts/%.dtb and dts/fsl/%.dtb to 'targets'. [1] [2]
> 
> cuImage.% should be file, but it does not cover all images.
> 
> Fix arch/powerpc/boot/Makefile.

Ah, thank you for the pointers, I did not quite get the meaning of those Makefile lines when first
reading them. So the issue is that I'm trying to build a cuImage that is not added to image-y in the
powerpc Makefile. It is unfortunate that this leads to a very confusing error message about the
missing dts directory.

I'll send a new patch if I come to the conclusion that I actually need the cuImage (for the ancient
TQM5200 which hasn't really been touched since 2011).

Regards,
Matthias


> 
> 
> 
> [1] https://github.com/torvalds/linux/blob/v6.7-rc5/arch/powerpc/boot/Makefile#L386
> [2] https://github.com/torvalds/linux/blob/v6.7-rc5/arch/powerpc/boot/Makefile#L388
> 
> 
> 
> >  scripts/Makefile.lib | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index 1a965fe68e011..3fe0fc46badfe 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -389,7 +389,8 @@ $(obj)/%.dtbo.S: $(obj)/%.dtbo FORCE
> >         $(call if_changed,wrap_S_dtb)
> > 
> >  quiet_cmd_dtc = DTC     $@
> > -cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
> > +cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
> > +       $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
> >         $(DTC) -o $@ -b 0 \
> >                 $(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
> >                 -d $(depfile).dtc.tmp $(dtc-tmp) ; \
> > --
> > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> > Amtsgericht München, HRB 105018
> > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> > https://www.tq-group.com/
> > 
> 
> 
> --
> Best Regards
> 
> Masahiro Yamada
>
Masahiro Yamada Dec. 15, 2023, 5:14 p.m. UTC | #3
On Wed, Dec 13, 2023 at 8:22 PM Matthias Schiffer
<matthias.schiffer@ew.tq-group.com> wrote:
>
> On Tue, 2023-12-12 at 17:13 +0000, Masahiro Yamada wrote:
> >
> >
> > On Wed, Dec 13, 2023 at 1:17 AM Matthias Schiffer
> > <matthias.schiffer@ew.tq-group.com> wrote:
> > >
> > > This reverts commit dd7699e37f289fa433f42c6bcc108468c8b198c0.
> > >
> > > On powerpc, dtb-y is usually empty unless CONFIG_OF_ALL_DTBS is set. While
> > > passing a DTB as a make target explicitly works fine, individual DTB
> > > builds may also be pulled in as dependencies by cuImage.% and similar
> > > targets. In this case, nothing creates the arch/powerpc/dts directory,
> > > causing out-of-tree builds to fail.
> > >
> > > Fixes: dd7699e37f28 ("Revert "kbuild: Create directory for target DTB"")
> > > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > > ---
> >
> >
> >
> > NACK.
> >
> > %.dtb is generated by if_changed_dep.
> >
> > Each Makefile is responsible for adding %.dtb to 'targets'
> > if it is pulled in as dependencies of other images.
> >
> > If it does not work for PowerPC, it is a bug in PowerPC Makefile.
> >
> >
> > Just checking arch/powerpc/boot/Makefile,
> > it adds dts/%.dtb and dts/fsl/%.dtb to 'targets'. [1] [2]
> >
> > cuImage.% should be file, but it does not cover all images.
> >
> > Fix arch/powerpc/boot/Makefile.
>
> Ah, thank you for the pointers, I did not quite get the meaning of those Makefile lines when first
> reading them. So the issue is that I'm trying to build a cuImage that is not added to image-y in the
> powerpc Makefile. It is unfortunate that this leads to a very confusing error message about the
> missing dts directory.
>
> I'll send a new patch if I come to the conclusion that I actually need the cuImage (for the ancient
> TQM5200 which hasn't really been touched since 2011).




If your target image does not exist in arch/powerpc/boot/Makefile,
one solution might be CONFIG_EXTRA_TARGETS


I see the following code:

  image-y += $(CONFIG_EXTRA_TARGETS)




Another solution might be:

  image-y += $(filter dtbImage.% uImage.% cuImage.% simpleImage.%
treeImage.%, $(MAKECMDGOALS))




But, I did not test any of them.






> Regards,
> Matthias
>
>
> >
> >
> >
> > [1] https://github.com/torvalds/linux/blob/v6.7-rc5/arch/powerpc/boot/Makefile#L386
> > [2] https://github.com/torvalds/linux/blob/v6.7-rc5/arch/powerpc/boot/Makefile#L388
> >
> >
> >
> > >  scripts/Makefile.lib | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > > index 1a965fe68e011..3fe0fc46badfe 100644
> > > --- a/scripts/Makefile.lib
> > > +++ b/scripts/Makefile.lib
> > > @@ -389,7 +389,8 @@ $(obj)/%.dtbo.S: $(obj)/%.dtbo FORCE
> > >         $(call if_changed,wrap_S_dtb)
> > >
> > >  quiet_cmd_dtc = DTC     $@
> > > -cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
> > > +cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
> > > +       $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
> > >         $(DTC) -o $@ -b 0 \
> > >                 $(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
> > >                 -d $(depfile).dtc.tmp $(dtc-tmp) ; \
> > > --
> > > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> > > Amtsgericht München, HRB 105018
> > > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> > > https://www.tq-group.com/
> > >
> >
> >
> > --
> > Best Regards
> >
> > Masahiro Yamada
> >
>
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> https://www.tq-group.com/
diff mbox series

Patch

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 1a965fe68e011..3fe0fc46badfe 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -389,7 +389,8 @@  $(obj)/%.dtbo.S: $(obj)/%.dtbo FORCE
 	$(call if_changed,wrap_S_dtb)
 
 quiet_cmd_dtc = DTC     $@
-cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
+cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
+	$(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
 	$(DTC) -o $@ -b 0 \
 		$(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
 		-d $(depfile).dtc.tmp $(dtc-tmp) ; \