Add the possibility to use another DTC
ClosedPublic

Authored by manu on Feb 13 2017, 9:10 PM.

Details

Summary

Add the possibility to use another DTC

Add a make.conf DTC variable that control which DTC (Device Tree Compiler) to use.

Test Plan

Compile kernel for ARM which uses DTC.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
manu updated this revision to Diff 25115.Feb 13 2017, 9:10 PM
manu retitled this revision from to Add the possibility to use another DTC.
manu updated this object.
manu edited the test plan for this revision. (Show Details)
manu added reviewers: imp, bdrewery, ARM.
manu set the repository for this revision to rS FreeBSD src repository.
manu added a project: ARM.
imp added inline comments.Feb 13 2017, 10:31 PM
sys/conf/kmod.mk
67 ↗(On Diff #25115)

Don't we need a .export here to pass the value to the subshell that does the compile?

bdrewery requested changes to this revision.Feb 13 2017, 10:36 PM
bdrewery edited edge metadata.
bdrewery added inline comments.
sys/conf/kmod.mk
67 ↗(On Diff #25115)

Yup, and please use "dtc" as the script had, not "/usr/bin/dtc". Otherwise we won't be calling one in the build tools directory, if we built one. Even if we don't do that now, it's better practice to support build tools directories being set via PATH.

sys/tools/fdt/make_dtb.sh
9 ↗(On Diff #25115)

I recommend adding a default DTC variable here in case none is sent from environment. This is done for MACHINE on line 15 as well but is simpler:

: ${DTC:=dtc}

This will set DTC to dtc if no value is set yet.

This revision now requires changes to proceed.Feb 13 2017, 10:36 PM
bdrewery added inline comments.Feb 13 2017, 10:40 PM
sys/conf/kmod.mk
67 ↗(On Diff #25115)

I hadn't checked yet, but we do build dtc as a build tool in bootstrap-tools. So we definitely don't want "/usr/bin/" here. That would make the manpage inaccurate too.

manu added a comment.Feb 13 2017, 10:56 PM

Yeah I definitely doesn't work for me as is (also I'm sure it did when I created this review but I must have had some local mods).
I can't seems to be able to use .export, are .export variabled really exported for subshell ? Where is this documented ?

manu updated this revision to Diff 25210.Feb 15 2017, 11:51 AM
manu edited edge metadata.
manu removed rS FreeBSD src repository as the repository for this revision.

Export the DTC variable in bsd.dtb.mk and correct man page.

wblock added a subscriber: wblock.Feb 17 2017, 5:38 PM
wblock added inline comments.
share/man/man5/make.conf.5
27 ↗(On Diff #25210)

Please remember to bump .Dd.

bdrewery requested changes to this revision.Feb 17 2017, 7:59 PM
bdrewery edited edge metadata.
bdrewery added inline comments.
sys/tools/fdt/make_dtb.sh
19–21 ↗(On Diff #25210)

This is redundant, you don't need the if and fi lines, just the middle ':' line is enough here, or make the middle line DTC=dtc.

This revision now requires changes to proceed.Feb 17 2017, 7:59 PM
manu updated this revision to Diff 25612.Feb 23 2017, 5:05 AM
manu edited edge metadata.

Update based on comment from from bdrewery@ and wblock@

manu marked 2 inline comments as done.Feb 23 2017, 5:05 AM
manu updated this revision to Diff 26244.Mar 14 2017, 8:31 AM

Update to latest HEAD

imp accepted this revision.Mar 14 2017, 4:01 PM

Looks like all of Bryan's feedback has been answered, and that answers the questions I had.

emaste added a subscriber: emaste.Jun 17 2017, 11:11 PM
This revision now requires changes to proceed.Jun 17 2017, 11:11 PM
bdrewery accepted this revision.Jun 17 2017, 11:29 PM
This revision is now accepted and ready to land.Jun 17 2017, 11:29 PM
This revision was automatically updated to reflect the committed changes.
wblock added inline comments.Jul 7 2017, 4:18 PM
head/share/man/man5/make.conf.5
27

Please bump this date.

183

This should be

is initially set to the value of
.Dq Li dtc .