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

Message ID 20210407193929.1251903-1-luis.machado@linaro.org
State New
Headers show
Series
  • [sim,moxie] Fix DTB generation mechanism and build failure
Related show

Commit Message

Keith Seitz via Gdb-patches April 7, 2021, 7:39 p.m.
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/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

Comments

Keith Seitz via Gdb-patches April 7, 2021, 8:26 p.m. | #1
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
Keith Seitz via Gdb-patches April 7, 2021, 8:39 p.m. | #2
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

>
Keith Seitz via Gdb-patches April 7, 2021, 10:31 p.m. | #3
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
Keith Seitz via Gdb-patches April 8, 2021, 2:40 p.m. | #4
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

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 ;