[PATCH] [sim,moxie] Fix DTB generation mechanism and build failure

[PATCH] [sim,moxie] Fix DTB generation mechanism and build failure

[Luis Machado via Gdb-patches]

I ran into a build failure with --enable-targets=all due to the fact that
the moxie sim expects to be able to use the dtc tool.  If it isn't available,
the builds fails.

Given the device tree compiler (dtc) is not available everywhere, it seems
fair to only generate the DTB file on the spot if we have such a tool.  For
those who don't have the tool available, we can use a prebuilt version of the
DTB available in the repository.

The DTS file hasn't changed since ~2009, so it seems pretty safe to assume
a prebuilt version is suitable to be used.

I also checked that the DTB file generated on an x86_64-Linux machine is the
the same as the one generated on an AArch64-Linux machine.

Tested by running make/make install with/without the dtc tool.
---
 sim/moxie/Makefile.in   |  12 ++++++++++--
 sim/moxie/moxie-gdb.dtb | Bin 0 -> 519 bytes
 2 files changed, 10 insertions(+), 2 deletions(-)
 create mode 100644 sim/moxie/moxie-gdb.dtb

diff --git a/sim/moxie/Makefile.in b/sim/moxie/Makefile.in
index ee513867290..65c41e6c2ac 100644
--- a/sim/moxie/Makefile.in
+++ b/sim/moxie/Makefile.in
@@ -17,6 +17,8 @@
 
 ## COMMON_PRE_CONFIG_FRAG
 
+DTC = @DTC@
+
 dtbdir = @datadir@/gdb/dtb
 
 SIM_OBJS = \
@@ -33,8 +35,14 @@ SIM_EXTRA_CFLAGS = -DDTB="\"$(dtbdir)/moxie-gdb.dtb\""
 all: moxie-gdb.dtb
 
 moxie-gdb.dtb: moxie-gdb.dts
-	dtc -O dtb -o moxie-gdb.dtb ${srcdir}/moxie-gdb.dts
+	if test "x$(DTC)" != x; then \
+	  $(DTC) -O dtb -o moxie-gdb.dtb ${srcdir}/moxie-gdb.dts ; \
+	fi ;
 
 install-dtb: moxie-gdb.dtb
 	$(SHELL) $(srcdir)/../../mkinstalldirs $(DESTDIR)$(dtbdir)
-	$(INSTALL_DATA) moxie-gdb.dtb $(DESTDIR)$(dtbdir)/moxie-gdb.dtb
+	if test "x$(DTC)" = x; then \
+	  $(INSTALL_DATA) $(srcdir)/moxie-gdb.dtb $(DESTDIR)$(dtbdir)/moxie-gdb.dtb ; \
+	else \
+	  $(INSTALL_DATA) moxie-gdb.dtb $(DESTDIR)$(dtbdir)/moxie-gdb.dtb ; \
+	fi ;
diff --git a/sim/moxie/moxie-gdb.dtb b/sim/moxie/moxie-gdb.dtb
new file mode 100644
index 0000000000000000000000000000000000000000..4c7e4570438a62b81df2021bb4c955d9888d2a8b
GIT binary patch
literal 519
zcmZ8d%}T^D5T34riikhgUWA3kLo4;>f(v``<U>fCjx~}tA<2sM>|^;5zKE|N&LrtB
z8ko*}U%r{nm#>qbZ-ChW0Nes(pOmjC&MD3)_&gB*5z9Z{ETKKh`>AGb!lzVE_=4)Z
zYn!6iZxTyXPbHz)#QH;ug-7S*&@r3!*lRnkt8|!S9q(DhJEj81y|nvwi5Zodc-9UF
zW`icDe5cII>V^hh3OzWjaD}y1qeEK-UF<U(7tqcJ!^sExy#I&UC!b81%{q<>|KvB!
zgWu%3x?V1WayPr69;VpaF~uV4x60`)gFm|G9j7>&*KKRjxl*7~4(3k2W2%MjP3>i)
d5PIFBf)-z;z(+qC1dAOsTKn2|)I}RO_yq~^OHTj*

literal 0
HcmV?d00001

-- 
2.25.1

Re: [PATCH] [sim,moxie] Fix DTB generation mechanism and build failure

[Mike Frysinger via Gdb-patches]

On 07 Apr 2021 16:39, Luis Machado via Gdb-patches wrote:
> I ran into a build failure with --enable-targets=all due to the fact that
> the moxie sim expects to be able to use the dtc tool.  If it isn't available,
> the builds fails.
> 
> Given the device tree compiler (dtc) is not available everywhere, it seems
> fair to only generate the DTB file on the spot if we have such a tool.  For
> those who don't have the tool available, we can use a prebuilt version of the
> DTB available in the repository.

i think i asked about vendoring the tool in, but i think people were
(understandably) not super keen on the idea since it's not GPL or GNU.

> The DTS file hasn't changed since ~2009, so it seems pretty safe to assume
> a prebuilt version is suitable to be used.

and because it's quite small.  if it were much larger, i don't think we'd
want to do it this way.

> I also checked that the DTB file generated on an x86_64-Linux machine is the
> the same as the one generated on an AArch64-Linux machine.

right, by design, it should be stable no matter where it's created.

> Tested by running make/make install with/without the dtc tool.

i think we want to fold this under the existing maintainer logic.  check out
commit 8c379db285f2ab8ad298288e86103548b90674a2 for a bit more detail.  we'd
want to only update+install the version in the source tree rather than switch
between the one in the srcdir & the builddir one.

also look at how move-if-change is used in common/Make-common.in.

lmk if that's too vague :).
-mike

Re: [PATCH] [sim,moxie] Fix DTB generation mechanism and build failure

[Luis Machado via Gdb-patches]

On 4/7/21 5:26 PM, Mike Frysinger wrote:
> On 07 Apr 2021 16:39, Luis Machado via Gdb-patches wrote:
>> I ran into a build failure with --enable-targets=all due to the fact that
>> the moxie sim expects to be able to use the dtc tool.  If it isn't available,
>> the builds fails.
>>
>> Given the device tree compiler (dtc) is not available everywhere, it seems
>> fair to only generate the DTB file on the spot if we have such a tool.  For
>> those who don't have the tool available, we can use a prebuilt version of the
>> DTB available in the repository.
> 
> i think i asked about vendoring the tool in, but i think people were
> (understandably) not super keen on the idea since it's not GPL or GNU.
> 
>> The DTS file hasn't changed since ~2009, so it seems pretty safe to assume
>> a prebuilt version is suitable to be used.
> 
> and because it's quite small.  if it were much larger, i don't think we'd
> want to do it this way.
> 
>> I also checked that the DTB file generated on an x86_64-Linux machine is the
>> the same as the one generated on an AArch64-Linux machine.
> 
> right, by design, it should be stable no matter where it's created.
> 
>> Tested by running make/make install with/without the dtc tool.
> 
> i think we want to fold this under the existing maintainer logic.  check out
> commit 8c379db285f2ab8ad298288e86103548b90674a2 for a bit more detail.  we'd
> want to only update+install the version in the source tree rather than switch
> between the one in the srcdir & the builddir one.

I'll take a look. Are you proposing we only use the prebuilt file from 
the source tree instead of generating a new dtb file during the build? 
And we should only do those things if maintainer mode is enabled?

> 
> also look at how move-if-change is used in common/Make-common.in.
> 
> lmk if that's too vague :).
> -mike
> 

Re: [PATCH] [sim,moxie] Fix DTB generation mechanism and build failure

[Mike Frysinger via Gdb-patches]

On 07 Apr 2021 17:39, Luis Machado via Gdb-patches wrote:
> On 4/7/21 5:26 PM, Mike Frysinger wrote:
> > On 07 Apr 2021 16:39, Luis Machado via Gdb-patches wrote:
> >> I ran into a build failure with --enable-targets=all due to the fact that
> >> the moxie sim expects to be able to use the dtc tool.  If it isn't available,
> >> the builds fails.
> >>
> >> Given the device tree compiler (dtc) is not available everywhere, it seems
> >> fair to only generate the DTB file on the spot if we have such a tool.  For
> >> those who don't have the tool available, we can use a prebuilt version of the
> >> DTB available in the repository.
> > 
> > i think i asked about vendoring the tool in, but i think people were
> > (understandably) not super keen on the idea since it's not GPL or GNU.
> > 
> >> The DTS file hasn't changed since ~2009, so it seems pretty safe to assume
> >> a prebuilt version is suitable to be used.
> > 
> > and because it's quite small.  if it were much larger, i don't think we'd
> > want to do it this way.
> > 
> >> I also checked that the DTB file generated on an x86_64-Linux machine is the
> >> the same as the one generated on an AArch64-Linux machine.
> > 
> > right, by design, it should be stable no matter where it's created.
> > 
> >> Tested by running make/make install with/without the dtc tool.
> > 
> > i think we want to fold this under the existing maintainer logic.  check out
> > commit 8c379db285f2ab8ad298288e86103548b90674a2 for a bit more detail.  we'd
> > want to only update+install the version in the source tree rather than switch
> > between the one in the srcdir & the builddir one.
> 
> I'll take a look. Are you proposing we only use the prebuilt file from 
> the source tree instead of generating a new dtb file during the build? 
> And we should only do those things if maintainer mode is enabled?

if we're going to commit the prebuilt file to the tree, then yes, we should
only be using that.  we have a lot of similar styles:
* configure from configure.ac
* Makefile.in/aclocal.m4 from Makefile.am
* nltvals.def from common/gennltvals.py

so if maintainer mode is enabled, and dtc is available, and the dts is newer
than the dtb, we'd update the dtb in the source repo.  anyone changing the
dts would be responsible for this.  as you noted, this is pretty rare atm,
so shouldn't be a big deal.
-mike

Re: [PATCH] [sim,moxie] Fix DTB generation mechanism and build failure

[Luis Machado via Gdb-patches]

On 4/7/21 7:31 PM, Mike Frysinger wrote:
> On 07 Apr 2021 17:39, Luis Machado via Gdb-patches wrote:
>> On 4/7/21 5:26 PM, Mike Frysinger wrote:
>>> On 07 Apr 2021 16:39, Luis Machado via Gdb-patches wrote:
>>>> I ran into a build failure with --enable-targets=all due to the fact that
>>>> the moxie sim expects to be able to use the dtc tool.  If it isn't available,
>>>> the builds fails.
>>>>
>>>> Given the device tree compiler (dtc) is not available everywhere, it seems
>>>> fair to only generate the DTB file on the spot if we have such a tool.  For
>>>> those who don't have the tool available, we can use a prebuilt version of the
>>>> DTB available in the repository.
>>>
>>> i think i asked about vendoring the tool in, but i think people were
>>> (understandably) not super keen on the idea since it's not GPL or GNU.
>>>
>>>> The DTS file hasn't changed since ~2009, so it seems pretty safe to assume
>>>> a prebuilt version is suitable to be used.
>>>
>>> and because it's quite small.  if it were much larger, i don't think we'd
>>> want to do it this way.
>>>
>>>> I also checked that the DTB file generated on an x86_64-Linux machine is the
>>>> the same as the one generated on an AArch64-Linux machine.
>>>
>>> right, by design, it should be stable no matter where it's created.
>>>
>>>> Tested by running make/make install with/without the dtc tool.
>>>
>>> i think we want to fold this under the existing maintainer logic.  check out
>>> commit 8c379db285f2ab8ad298288e86103548b90674a2 for a bit more detail.  we'd
>>> want to only update+install the version in the source tree rather than switch
>>> between the one in the srcdir & the builddir one.
>>
>> I'll take a look. Are you proposing we only use the prebuilt file from
>> the source tree instead of generating a new dtb file during the build?
>> And we should only do those things if maintainer mode is enabled?
> 
> if we're going to commit the prebuilt file to the tree, then yes, we should
> only be using that.  we have a lot of similar styles:
> * configure from configure.ac
> * Makefile.in/aclocal.m4 from Makefile.am
> * nltvals.def from common/gennltvals.py
> 
> so if maintainer mode is enabled, and dtc is available, and the dts is newer
> than the dtb, we'd update the dtb in the source repo.  anyone changing the
> dts would be responsible for this.  as you noted, this is pretty rare atm,
> so shouldn't be a big deal.
> -mike
> 

Thanks for clarifying it. I've sent v2 now. Hopefully that has the right 
logic.

[PATCH,v2][sim,moxie] Fix DTB generation mechanism and build failure

[Luis Machado via Gdb-patches]

I ran into a build failure with --enable-targets=all due to the fact that
the moxie sim expects to be able to use the dtc tool.  If it isn't available,
the builds fails.

The following patch adds a prebuilt dtb file to the tree. That file is the one
that is used for installations.

The patch also enables (re-)generation of the dtb file through maintainer
mode, if it needs to be updated due to a change in the dts file.

Tested on aarch64-linux/x86_64-linux.

sim/moxie/ChangeLog:

YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>

	* Makefile.in (moxie-gdb.dtb): Add maintainer mode dependency.
	(install-dtb): Install prebuilt dtb file.
	* moxie-gdb.dtb: New prebuilt file.
---
 sim/moxie/Makefile.in   |  11 ++++++++---
 sim/moxie/moxie-gdb.dtb | Bin 0 -> 519 bytes
 2 files changed, 8 insertions(+), 3 deletions(-)
 create mode 100644 sim/moxie/moxie-gdb.dtb

diff --git a/sim/moxie/Makefile.in b/sim/moxie/Makefile.in
index ee513867290..78e140fdfb2 100644
--- a/sim/moxie/Makefile.in
+++ b/sim/moxie/Makefile.in
@@ -17,6 +17,8 @@
 
 ## COMMON_PRE_CONFIG_FRAG
 
+DTC = @DTC@
+
 dtbdir = @datadir@/gdb/dtb
 
 SIM_OBJS = \
@@ -32,9 +34,12 @@ SIM_EXTRA_CFLAGS = -DDTB="\"$(dtbdir)/moxie-gdb.dtb\""
 
 all: moxie-gdb.dtb
 
-moxie-gdb.dtb: moxie-gdb.dts
-	dtc -O dtb -o moxie-gdb.dtb ${srcdir}/moxie-gdb.dts
+moxie-gdb.dtb: @MAINTAINER_MODE_TRUE@ moxie-gdb.dts
+	if test "x$(DTC)" != x; then \
+	  $(DTC) -O dtb -o moxie-gdb.dtb ${srcdir}/moxie-gdb.dts ; \
+	  $(SHELL) $(srcroot)/move-if-change moxie-gdb.dtb ${srcdir}/moxie-gdb.dtb ; \
+	fi ;
 
 install-dtb: moxie-gdb.dtb
 	$(SHELL) $(srcdir)/../../mkinstalldirs $(DESTDIR)$(dtbdir)
-	$(INSTALL_DATA) moxie-gdb.dtb $(DESTDIR)$(dtbdir)/moxie-gdb.dtb
+	$(INSTALL_DATA) $(srcdir)/moxie-gdb.dtb $(DESTDIR)$(dtbdir)/moxie-gdb.dtb
diff --git a/sim/moxie/moxie-gdb.dtb b/sim/moxie/moxie-gdb.dtb
new file mode 100644
index 0000000000000000000000000000000000000000..4c7e4570438a62b81df2021bb4c955d9888d2a8b
GIT binary patch
literal 519
zcmZ8d%}T^D5T34riikhgUWA3kLo4;>f(v``<U>fCjx~}tA<2sM>|^;5zKE|N&LrtB
z8ko*}U%r{nm#>qbZ-ChW0Nes(pOmjC&MD3)_&gB*5z9Z{ETKKh`>AGb!lzVE_=4)Z
zYn!6iZxTyXPbHz)#QH;ug-7S*&@r3!*lRnkt8|!S9q(DhJEj81y|nvwi5Zodc-9UF
zW`icDe5cII>V^hh3OzWjaD}y1qeEK-UF<U(7tqcJ!^sExy#I&UC!b81%{q<>|KvB!
zgWu%3x?V1WayPr69;VpaF~uV4x60`)gFm|G9j7>&*KKRjxl*7~4(3k2W2%MjP3>i)
d5PIFBf)-z;z(+qC1dAOsTKn2|)I}RO_yq~^OHTj*

literal 0
HcmV?d00001

-- 
2.25.1

Re: [PATCH,v2][sim,moxie] Fix DTB generation mechanism and build failure

[Mike Frysinger via Gdb-patches]

On 08 Apr 2021 11:40, Luis Machado wrote:
> -moxie-gdb.dtb: moxie-gdb.dts
> -	dtc -O dtb -o moxie-gdb.dtb ${srcdir}/moxie-gdb.dts
> +moxie-gdb.dtb: @MAINTAINER_MODE_TRUE@ moxie-gdb.dts
> +	if test "x$(DTC)" != x; then \
> +	  $(DTC) -O dtb -o moxie-gdb.dtb ${srcdir}/moxie-gdb.dts ; \

needs `|| exit 1` to catch failures

> +	  $(SHELL) $(srcroot)/move-if-change moxie-gdb.dtb ${srcdir}/moxie-gdb.dtb ; \

same here

> +	fi ;

can you add a warning as an else case that the file has not been updated due
to missing dtc program ?  and delete this trailing ;

otherwise, lgtm!
-mike

[PATCH,v3][sim,moxie] Fix DTB generation mechanism and build failure

[Luis Machado via Gdb-patches]

I ran into a build failure with --enable-targets=all due to the fact that
the moxie sim expects to be able to use the dtc tool.  If it isn't available,
the builds fails.

The following patch adds a prebuilt dtb file to the tree. That file is the one
that is used for installations.

The patch also enables (re-)generation of the dtb file through maintainer
mode, if it needs to be updated due to a change in the dts file.

Tested on aarch64-linux/x86_64-linux.

sim/moxie/ChangeLog:

YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>

	* Makefile.in (moxie-gdb.dtb): Add maintainer mode dependency.
	(install-dtb): Install prebuilt dtb file.
	* moxie-gdb.dtb: New prebuilt file.
---
 sim/moxie/Makefile.in   |  16 +++++++++++++---
 sim/moxie/moxie-gdb.dtb | Bin 0 -> 519 bytes
 2 files changed, 13 insertions(+), 3 deletions(-)
 create mode 100644 sim/moxie/moxie-gdb.dtb

diff --git a/sim/moxie/Makefile.in b/sim/moxie/Makefile.in
index ee513867290..bd58074491f 100644
--- a/sim/moxie/Makefile.in
+++ b/sim/moxie/Makefile.in
@@ -17,6 +17,8 @@
 
 ## COMMON_PRE_CONFIG_FRAG
 
+DTC = @DTC@
+
 dtbdir = @datadir@/gdb/dtb
 
 SIM_OBJS = \
@@ -32,9 +34,17 @@ SIM_EXTRA_CFLAGS = -DDTB="\"$(dtbdir)/moxie-gdb.dtb\""
 
 all: moxie-gdb.dtb
 
-moxie-gdb.dtb: moxie-gdb.dts
-	dtc -O dtb -o moxie-gdb.dtb ${srcdir}/moxie-gdb.dts
+moxie-gdb.dtb: @MAINTAINER_MODE_TRUE@ moxie-gdb.dts
+	@echo Regenerating moxie-gdb.dtb
+	if test "x$(DTC)" != x; then \
+	  $(DTC) -O dtb -o moxie-gdb.dtb ${srcdir}/moxie-gdb.dts || exit 1; \
+	  $(SHELL) $(srcroot)/move-if-change moxie-gdb.dtb ${srcdir}/moxie-gdb.dtb || exit 1; \
+	else \
+	  echo "Could not update the moxie-gdb.dtb file because the device "; \
+	  echo "tree compiler tool (dtc) is missing.  Install the tool to "; \
+	  echo "update the device tree blob."; \
+	fi
 
 install-dtb: moxie-gdb.dtb
 	$(SHELL) $(srcdir)/../../mkinstalldirs $(DESTDIR)$(dtbdir)
-	$(INSTALL_DATA) moxie-gdb.dtb $(DESTDIR)$(dtbdir)/moxie-gdb.dtb
+	$(INSTALL_DATA) $(srcdir)/moxie-gdb.dtb $(DESTDIR)$(dtbdir)/moxie-gdb.dtb
diff --git a/sim/moxie/moxie-gdb.dtb b/sim/moxie/moxie-gdb.dtb
new file mode 100644
index 0000000000000000000000000000000000000000..4c7e4570438a62b81df2021bb4c955d9888d2a8b
GIT binary patch
literal 519
zcmZ8d%}T^D5T34riikhgUWA3kLo4;>f(v``<U>fCjx~}tA<2sM>|^;5zKE|N&LrtB
z8ko*}U%r{nm#>qbZ-ChW0Nes(pOmjC&MD3)_&gB*5z9Z{ETKKh`>AGb!lzVE_=4)Z
zYn!6iZxTyXPbHz)#QH;ug-7S*&@r3!*lRnkt8|!S9q(DhJEj81y|nvwi5Zodc-9UF
zW`icDe5cII>V^hh3OzWjaD}y1qeEK-UF<U(7tqcJ!^sExy#I&UC!b81%{q<>|KvB!
zgWu%3x?V1WayPr69;VpaF~uV4x60`)gFm|G9j7>&*KKRjxl*7~4(3k2W2%MjP3>i)
d5PIFBf)-z;z(+qC1dAOsTKn2|)I}RO_yq~^OHTj*

literal 0
HcmV?d00001

-- 
2.25.1

Re: [PATCH,v3][sim,moxie] Fix DTB generation mechanism and build failure

[Mike Frysinger via Gdb-patches]

On 08 Apr 2021 14:21, Luis Machado wrote:
> I ran into a build failure with --enable-targets=all due to the fact that
> the moxie sim expects to be able to use the dtc tool.  If it isn't available,
> the builds fails.
> 
> The following patch adds a prebuilt dtb file to the tree. That file is the one
> that is used for installations.
> 
> The patch also enables (re-)generation of the dtb file through maintainer
> mode, if it needs to be updated due to a change in the dts file.

lgtm, thanks!
-mike

Re: [PATCH,v3][sim,moxie] Fix DTB generation mechanism and build failure

[Luis Machado via Gdb-patches]

On 4/8/21 2:44 PM, Mike Frysinger wrote:
> On 08 Apr 2021 14:21, Luis Machado wrote:
>> I ran into a build failure with --enable-targets=all due to the fact that
>> the moxie sim expects to be able to use the dtc tool.  If it isn't available,
>> the builds fails.
>>
>> The following patch adds a prebuilt dtb file to the tree. That file is the one
>> that is used for installations.
>>
>> The patch also enables (re-)generation of the dtb file through maintainer
>> mode, if it needs to be updated due to a change in the dts file.
> 
> lgtm, thanks!
> -mike
> 

Great. Thanks for all the feedback. Pushed now.