[PATCH V8] kbuild: create a rule to run the pre-processor on *.dts files

[PATCH V8] kbuild: create a rule to run the pre-processor on *.dts files

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Create cmd_dtc_cpp to run the C pre-processor on *.dts file before
passing them to dtc for final compilation. This allows the use of #define
and #include within the .dts file.

Acked-by: Simon Glass <sjg@chromium.org>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Acked-by: Michal Marek <mmarek@suse.cz>
Acked-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
Grant, back in mid-November, you said you'd make a decision on this in
the next couple of days, but I think this got overlooked.

v8: Add -undef to the cpp command-line, to prevent e.g. "linux" and "unix"
    being defined; symbols which can appear in the device tree.
v7: Build *.dtb from *.dts not src/*.dts.
v6: No change.
v5:
* Update Documentation/kbuild for the new command and rule.
v4:
* Use -x assembler-with-cpp so pre-defined macros are set up so that
  #included header files know to only use cpp syntax, not C syntax.
* Define __DTS__ for similar reasons.
* use $(CPP) not $(CC) -E, and use $(cpp_flags).
* Save the pre-processed results so they can be easily inspected when
  debugging build issues.
* The use of -x assembler-with-cpp causes cpp to recognize directives in
  column 1 only. Hence, there's no need to escape property names that
  begin with #. Hence, there's no need for separate skeleton.dtsi and
  skeleton.dtsip. Maintain a separate file extension and build rule so that
  CPP-usage is opt-in. In particular, when using CPP, #include must be used
  rather than /include/ so that dependencies work.
v3: Pass "-x c" not "-xc" to cpp.
v2: Place make %.dtb: %.dtsp rule into Makefile.lib.
---
 Documentation/kbuild/makefiles.txt |   23 +++++++++++++++++++++++
 scripts/Makefile.lib               |   10 ++++++++++
 2 files changed, 33 insertions(+)

diff --git a/Documentation/kbuild/makefiles.txt b/Documentation/kbuild/makefiles.txt
index 14c3f4f..5198b74 100644
--- a/Documentation/kbuild/makefiles.txt
+++ b/Documentation/kbuild/makefiles.txt
@@ -1186,6 +1186,29 @@ When kbuild executes, the following steps are followed (roughly):
 		clean-files += *.dtb
 		DTC_FLAGS ?= -p 1024
 
+    dtc_cpp
+	This is just like dtc as describe above, except that the C pre-
+	processor is invoked upon the .dtsp file before compiling the result
+	with dtc.
+
+	In order for build dependencies to work, all files compiled using
+	dtc_cpp must use the C pre-processor's #include functionality and not
+	dtc's /include/ functionality.
+
+	Using the C pre-processor allows use of #define to create named
+	constants. In turn, the #defines will typically appear in a header
+	file, which may be shared with regular C code. Since the dtc language
+	represents a data structure rather than code in C syntax, similar
+	restrictions are placed on a header file included by a device tree
+	file as for a header file included by an assembly language file.
+	In particular, the C pre-processor is passed -x assembler-with-cpp,
+	which sets macro __ASSEMBLY__. __DTS__ is also set. These allow header
+	files to restrict their content to that compatible with device tree
+	source.
+
+	A central rule exists to create $(obj)/%.dtb from $(src)/%.dtsp;
+	architecture Makefiles do no need to explicitly write out that rule.
+
 --- 6.8 Custom kbuild commands
 
 	When kbuild is executing with KBUILD_VERBOSE=0, then only a shorthand
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index bdf42fd..7910229 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -269,6 +269,16 @@ cmd_dtc = $(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 $(DTC_FLAGS) -d $(depfile
 $(obj)/%.dtb: $(src)/%.dts FORCE
 	$(call if_changed_dep,dtc)
 
+dtc-tmp = $(subst $(comma),_,$(dot-target).dts)
+
+quiet_cmd_dtc_cpp = DTC+CPP $@
+cmd_dtc_cpp = $(CPP) $(cpp_flags) -x assembler-with-cpp -undef -D__DTS__  \
+			-o $(dtc-tmp) $< ; \
+	$(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 $(DTC_FLAGS) $(dtc-tmp)
+
+$(obj)/%.dtb: $(src)/%.dtsp FORCE
+	$(call if_changed_dep,dtc_cpp)
+
 # Bzip2
 # ---------------------------------------------------------------------------
 
-- 
1.7.10.4

Re: [PATCH V8] kbuild: create a rule to run the pre-processor on *.dts files

[Grant Likely]

On Tue,  5 Feb 2013 12:06:28 -0700, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> Create cmd_dtc_cpp to run the C pre-processor on *.dts file before
> passing them to dtc for final compilation. This allows the use of #define
> and #include within the .dts file.
> 
> Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> Acked-by: Michal Marek <mmarek-AlSwsSmVLrQ@public.gmane.org>
> Acked-by: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>
> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

I've applied this and was going to push it out, but I've just thought of
a problem that could be a show stopper. Once a dtsp file includes a C
header, the contents of that header become part of the Device Tree ABI.
If someone changes that file (ie. to renumber a series of #defines) then
that will break the binding. We need a way to protect against that.
Someone changing a .h file may make the assumption that it is only
kernel internal and won't realize that it has external implications.

I'm thinking that any dts includes need to be treated in the same way as
userspace headers. We could put them into include/uapi and piggy back on
the protection already afforded by that directory, or come up with
something new. Any thoughts?

g.

Re: [PATCH V8] kbuild: create a rule to run the pre-processor on *.dts files

[Stephen Warren]

On 02/08/2013 07:45 AM, Grant Likely wrote:
> On Tue,  5 Feb 2013 12:06:28 -0700, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> Create cmd_dtc_cpp to run the C pre-processor on *.dts file before
>> passing them to dtc for final compilation. This allows the use of #define
>> and #include within the .dts file.
>>
>> Acked-by: Simon Glass <sjg@chromium.org>
>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>> Acked-by: Michal Marek <mmarek@suse.cz>
>> Acked-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> 
> I've applied this and was going to push it out, but I've just thought of
> a problem that could be a show stopper. Once a dtsp file includes a C
> header, the contents of that header become part of the Device Tree ABI.
> If someone changes that file (ie. to renumber a series of #defines) then
> that will break the binding. We need a way to protect against that.
> Someone changing a .h file may make the assumption that it is only
> kernel internal and won't realize that it has external implications.
> 
> I'm thinking that any dts includes need to be treated in the same way as
> userspace headers. We could put them into include/uapi and piggy back on
> the protection already afforded by that directory, or come up with
> something new. Any thoughts?

Yes, that's true.

What protection is provided by include/uapi - something programmatic or
simply stricter review?

I wondered if we shouldn't put the headers into the .dts directory, then
update the kernel to include them too. That would also make it obvious
they were part of the DT ABI.

Re: [PATCH V8] kbuild: create a rule to run the pre-processor on *.dts files

[Grant Likely]

On Fri, Feb 8, 2013 at 5:18 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 02/08/2013 07:45 AM, Grant Likely wrote:
>> On Tue,  5 Feb 2013 12:06:28 -0700, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> From: Stephen Warren <swarren@nvidia.com>
>>>
>>> Create cmd_dtc_cpp to run the C pre-processor on *.dts file before
>>> passing them to dtc for final compilation. This allows the use of #define
>>> and #include within the .dts file.
>>>
>>> Acked-by: Simon Glass <sjg@chromium.org>
>>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>> Acked-by: Michal Marek <mmarek@suse.cz>
>>> Acked-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>
>> I've applied this and was going to push it out, but I've just thought of
>> a problem that could be a show stopper. Once a dtsp file includes a C
>> header, the contents of that header become part of the Device Tree ABI.
>> If someone changes that file (ie. to renumber a series of #defines) then
>> that will break the binding. We need a way to protect against that.
>> Someone changing a .h file may make the assumption that it is only
>> kernel internal and won't realize that it has external implications.
>>
>> I'm thinking that any dts includes need to be treated in the same way as
>> userspace headers. We could put them into include/uapi and piggy back on
>> the protection already afforded by that directory, or come up with
>> something new. Any thoughts?
>
> Yes, that's true.
>
> What protection is provided by include/uapi - something programmatic or
> simply stricter review?

I believe there are tools that parse the userspace ABI and flag
incompatibilities, but also stricter review.

> I wondered if we shouldn't put the headers into the .dts directory, then
> update the kernel to include them too. That would also make it obvious
> they were part of the DT ABI.

That would also help be good. In either case the CPP options need to
only include the ABI headers. No normal headers allowed.

g.



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH V8] kbuild: create a rule to run the pre-processor on *.dts files

[Rob Herring]

On 02/08/2013 08:45 AM, Grant Likely wrote:
> On Tue,  5 Feb 2013 12:06:28 -0700, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> Create cmd_dtc_cpp to run the C pre-processor on *.dts file before
>> passing them to dtc for final compilation. This allows the use of #define
>> and #include within the .dts file.
>>
>> Acked-by: Simon Glass <sjg@chromium.org>
>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>> Acked-by: Michal Marek <mmarek@suse.cz>
>> Acked-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> 
> I've applied this and was going to push it out, but I've just thought of
> a problem that could be a show stopper. Once a dtsp file includes a C
> header, the contents of that header become part of the Device Tree ABI.
> If someone changes that file (ie. to renumber a series of #defines) then
> that will break the binding. We need a way to protect against that.
> Someone changing a .h file may make the assumption that it is only
> kernel internal and won't realize that it has external implications.
> 
> I'm thinking that any dts includes need to be treated in the same way as
> userspace headers. We could put them into include/uapi and piggy back on
> the protection already afforded by that directory, or come up with
> something new. Any thoughts?

Also, we would never be able to separate the dts files from the kernel
tree without some separation.

Rob

Re: [PATCH V8] kbuild: create a rule to run the pre-processor on *.dts files

[Grant Likely]

On Fri, 08 Feb 2013 15:24:25 -0600, Rob Herring <robherring2@gmail.com> wrote:
> On 02/08/2013 08:45 AM, Grant Likely wrote:
> > On Tue,  5 Feb 2013 12:06:28 -0700, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >> From: Stephen Warren <swarren@nvidia.com>
> >>
> >> Create cmd_dtc_cpp to run the C pre-processor on *.dts file before
> >> passing them to dtc for final compilation. This allows the use of #define
> >> and #include within the .dts file.
> >>
> >> Acked-by: Simon Glass <sjg@chromium.org>
> >> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> >> Acked-by: Michal Marek <mmarek@suse.cz>
> >> Acked-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
> >> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> > 
> > I've applied this and was going to push it out, but I've just thought of
> > a problem that could be a show stopper. Once a dtsp file includes a C
> > header, the contents of that header become part of the Device Tree ABI.
> > If someone changes that file (ie. to renumber a series of #defines) then
> > that will break the binding. We need a way to protect against that.
> > Someone changing a .h file may make the assumption that it is only
> > kernel internal and won't realize that it has external implications.
> > 
> > I'm thinking that any dts includes need to be treated in the same way as
> > userspace headers. We could put them into include/uapi and piggy back on
> > the protection already afforded by that directory, or come up with
> > something new. Any thoughts?
> 
> Also, we would never be able to separate the dts files from the kernel
> tree without some separation.

Good point. They better be in a completely separate directory then.

g.

Re: [PATCH V8] kbuild: create a rule to run the pre-processor on *.dts files

[Stephen Warren]

On 02/08/2013 03:04 PM, Grant Likely wrote:
> On Fri, 08 Feb 2013 15:24:25 -0600, Rob Herring <robherring2@gmail.com> wrote:
>> On 02/08/2013 08:45 AM, Grant Likely wrote:
>>> On Tue,  5 Feb 2013 12:06:28 -0700, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> From: Stephen Warren <swarren@nvidia.com>
>>>>
>>>> Create cmd_dtc_cpp to run the C pre-processor on *.dts file before
>>>> passing them to dtc for final compilation. This allows the use of #define
>>>> and #include within the .dts file.
>>>>
>>>> Acked-by: Simon Glass <sjg@chromium.org>
>>>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>>> Acked-by: Michal Marek <mmarek@suse.cz>
>>>> Acked-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>>
>>> I've applied this and was going to push it out, but I've just thought of
>>> a problem that could be a show stopper. Once a dtsp file includes a C
>>> header, the contents of that header become part of the Device Tree ABI.
>>> If someone changes that file (ie. to renumber a series of #defines) then
>>> that will break the binding. We need a way to protect against that.
>>> Someone changing a .h file may make the assumption that it is only
>>> kernel internal and won't realize that it has external implications.
>>>
>>> I'm thinking that any dts includes need to be treated in the same way as
>>> userspace headers. We could put them into include/uapi and piggy back on
>>> the protection already afforded by that directory, or come up with
>>> something new. Any thoughts?
>>
>> Also, we would never be able to separate the dts files from the kernel
>> tree without some separation.
> 
> Good point. They better be in a completely separate directory then.

How about arch/*/include/dts/.

Both *.dts and kernel code will need to include those headers (so that
the kernel can parse the DTB using the same values), so putting the
headers right into arch/*/boot/dts/ or arch/*/boot/dts/include/ might
not make sense, since then that directory would still have to exist even
if/when the .dts files are separated from the kernel.

Re: [PATCH V8] kbuild: create a rule to run the pre-processor on *.dts files

[Stephen Warren]

On 02/08/2013 08:45 AM, Grant Likely wrote:
> On Tue,  5 Feb 2013 12:06:28 -0700, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> Create cmd_dtc_cpp to run the C pre-processor on *.dts file before
>> passing them to dtc for final compilation. This allows the use of #define
>> and #include within the .dts file.
...
> I've applied this and was going to push it out, but I've just thought of
> a problem that could be a show stopper. ...

Grant, I notice this showed up in next-20130211 as:

22435f3 kbuild: create a rule to run the pre-processor on *.dts files

Was that intentional? I assumed you wanted me to rework this before
you'd apply it, rather than sending an incremental patch.

Re: [PATCH V8] kbuild: create a rule to run the pre-processor on *.dts files

[Grant Likely]

On Mon, 11 Feb 2013 12:11:22 -0700, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> On 02/08/2013 08:45 AM, Grant Likely wrote:
> > On Tue,  5 Feb 2013 12:06:28 -0700, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> >> Create cmd_dtc_cpp to run the C pre-processor on *.dts file before
> >> passing them to dtc for final compilation. This allows the use of #define
> >> and #include within the .dts file.
> ...
> > I've applied this and was going to push it out, but I've just thought of
> > a problem that could be a show stopper. ...
> 
> Grant, I notice this showed up in next-20130211 as:
> 
> 22435f3 kbuild: create a rule to run the pre-processor on *.dts files
> 
> Was that intentional? I assumed you wanted me to rework this before
> you'd apply it, rather than sending an incremental patch.

Yes it was intentional. I sat on my hands w.r.t. this patch for long
enough that I didn't want to delay it any longer. However, I still want
a solution to the problem discussed above before it becomes a
maintenance disaster.

g.


-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

Re: [PATCH V8] kbuild: create a rule to run the pre-processor on *.dts files

[Stephen Warren]

On 02/12/2013 11:08 AM, Grant Likely wrote:
> On Mon, 11 Feb 2013 12:11:22 -0700, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 02/08/2013 08:45 AM, Grant Likely wrote:
>>> On Tue,  5 Feb 2013 12:06:28 -0700, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> Create cmd_dtc_cpp to run the C pre-processor on *.dts file before
>>>> passing them to dtc for final compilation. This allows the use of #define
>>>> and #include within the .dts file.
>> ...
>>> I've applied this and was going to push it out, but I've just thought of
>>> a problem that could be a show stopper. ...
>>
>> Grant, I notice this showed up in next-20130211 as:
>>
>> 22435f3 kbuild: create a rule to run the pre-processor on *.dts files
>>
>> Was that intentional? I assumed you wanted me to rework this before
>> you'd apply it, rather than sending an incremental patch.
> 
> Yes it was intentional. I sat on my hands w.r.t. this patch for long
> enough that I didn't want to delay it any longer. However, I still want
> a solution to the problem discussed above before it becomes a
> maintenance disaster.

OK, I'll send an incremental patch to adjust the include paths. It
should be soon; I think I have it written, but am distracted fixing up
Tegra clock driver issues etc.:-(