diff mbox

kbuild: create an "include chroot" for DT bindings

Message ID 1361394356-19385-1-git-send-email-swarren@wwwdotorg.org (mailing list archive)
State New, archived
Headers show

Commit Message

Stephen Warren Feb. 20, 2013, 9:05 p.m. UTC
From: Stephen Warren <swarren@nvidia.com>

The recent dtc+cpp support allows header files and C pre-processor
defines/macros to be used when compiling device tree files. These
headers will typically define various constants that are part of the
device tree bindings.

The original patch which set up the dtc+cpp include path only considered
using those headers from device tree files. However, they are also
useful for kernel code which needs to interpret the device tree.

In both the DT files and the kernel, I'd like to include the DT-related
headers in the same way, for example, <dt-bindings/gpio/tegra-gpio.h>.
That will simplify any text which discusses the DT header locations.

Creating a <dt-bindings/> for kernel source to use is as simple as
placing files into include/dt-bindings/.

However, when compiling DT files, the include path should be restricted
so that only the dt-bindings path is available; arbitrary kernel headers
shouldn't be exposed. For this reason, create a specific include
directory for use by dtc+cpp, and symlink dt-bindings from there to the
actual location of include/dt-bindings/. For want of a better location,
place this "include chroot" into the existing dts/ directory.

arch/*/boot/dts/include/dt-bindings -> ../../../../../dt-bindings

Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Hiroshi Doyu <hdoyu@nvidia.com>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
You can find an example of how this would be used at
git://nv-tegra.nvidia.com/user/swarren/linux-2.6 linux-next_common
(note: there's a bunch of other cruft in that branch too)

 arch/arm/boot/dts/include/dt-bindings |    1 +
 scripts/Makefile.lib                  |    3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)
 create mode 120000 arch/arm/boot/dts/include/dt-bindings

Comments

Shawn Guo Feb. 21, 2013, 4:37 a.m. UTC | #1
On Wed, Feb 20, 2013 at 02:05:56PM -0700, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> The recent dtc+cpp support allows header files and C pre-processor
> defines/macros to be used when compiling device tree files. These
> headers will typically define various constants that are part of the
> device tree bindings.
> 
> The original patch which set up the dtc+cpp include path only considered
> using those headers from device tree files. However, they are also
> useful for kernel code which needs to interpret the device tree.
> 
> In both the DT files and the kernel, I'd like to include the DT-related
> headers in the same way, for example, <dt-bindings/gpio/tegra-gpio.h>.
> That will simplify any text which discusses the DT header locations.
> 
> Creating a <dt-bindings/> for kernel source to use is as simple as
> placing files into include/dt-bindings/.
> 
> However, when compiling DT files, the include path should be restricted
> so that only the dt-bindings path is available; arbitrary kernel headers
> shouldn't be exposed. For this reason, create a specific include
> directory for use by dtc+cpp, and symlink dt-bindings from there to the
> actual location of include/dt-bindings/. For want of a better location,
> place this "include chroot" into the existing dts/ directory.
> 
> arch/*/boot/dts/include/dt-bindings -> ../../../../../dt-bindings

../../../../../include/dt-bindings

And, I would expect only the headers that will actually be referenced
by kernel should go into include/dt-bindings.  There is no point to
put headers that will not be included by kernel but only by dts files
into there, and instead arch/arm/boot/dts/include should just be good
enough for them.  With this agreed ...

> Cc: Shawn Guo <shawn.guo@linaro.org>

Acked-by: Shawn Guo <shawn.guo@linaro.org>

> Cc: Hiroshi Doyu <hdoyu@nvidia.com>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> You can find an example of how this would be used at
> git://nv-tegra.nvidia.com/user/swarren/linux-2.6 linux-next_common
> (note: there's a bunch of other cruft in that branch too)
> 
>  arch/arm/boot/dts/include/dt-bindings |    1 +
>  scripts/Makefile.lib                  |    3 +--
>  2 files changed, 2 insertions(+), 2 deletions(-)
>  create mode 120000 arch/arm/boot/dts/include/dt-bindings
> 
> diff --git a/arch/arm/boot/dts/include/dt-bindings b/arch/arm/boot/dts/include/dt-bindings
> new file mode 120000
> index 0000000..08c00e4
> --- /dev/null
> +++ b/arch/arm/boot/dts/include/dt-bindings
> @@ -0,0 +1 @@
> +../../../../../include/dt-bindings
> \ No newline at end of file
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 07125e6..d6c5036 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -157,8 +157,7 @@ cpp_flags      = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE)     \
>  ld_flags       = $(LDFLAGS) $(ldflags-y)
>  
>  dtc_cpp_flags  = -Wp,-MD,$(depfile) -nostdinc                            \
> -		 -I$(srctree)/arch/$(SRCARCH)/boot/dts                   \
> -		 -I$(srctree)/arch/$(SRCARCH)/include/dts                \
> +		 -I$(srctree)/arch/$(SRCARCH)/boot/dts/include           \
>  		 -undef -D__DTS__
>  
>  # Finds the multi-part object the current object will be linked into
> -- 
> 1.7.10.4
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren Feb. 21, 2013, 6:43 p.m. UTC | #2
On 02/20/2013 09:37 PM, Shawn Guo wrote:
> On Wed, Feb 20, 2013 at 02:05:56PM -0700, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> The recent dtc+cpp support allows header files and C pre-processor
>> defines/macros to be used when compiling device tree files. These
>> headers will typically define various constants that are part of the
>> device tree bindings.
>>
>> The original patch which set up the dtc+cpp include path only considered
>> using those headers from device tree files. However, they are also
>> useful for kernel code which needs to interpret the device tree.
>>
>> In both the DT files and the kernel, I'd like to include the DT-related
>> headers in the same way, for example, <dt-bindings/gpio/tegra-gpio.h>.
>> That will simplify any text which discusses the DT header locations.
>>
>> Creating a <dt-bindings/> for kernel source to use is as simple as
>> placing files into include/dt-bindings/.
>>
>> However, when compiling DT files, the include path should be restricted
>> so that only the dt-bindings path is available; arbitrary kernel headers
>> shouldn't be exposed. For this reason, create a specific include
>> directory for use by dtc+cpp, and symlink dt-bindings from there to the
>> actual location of include/dt-bindings/. For want of a better location,
>> place this "include chroot" into the existing dts/ directory.
>>
>> arch/*/boot/dts/include/dt-bindings -> ../../../../../dt-bindings
> 
> ../../../../../include/dt-bindings

Yup.

> And, I would expect only the headers that will actually be referenced
> by kernel should go into include/dt-bindings.  There is no point to
> put headers that will not be included by kernel but only by dts files
> into there, and instead arch/arm/boot/dts/include should just be good
> enough for them.  With this agreed ...

There are two things that include DT-related headers:

a) Device trees (*.dts, *.dtsi)
b) The kernel

All the headers relevant here fall into category (a) by definition. I'd
actually expect most to also fall into category (b), although I can see
that category (b) might be a strict subset of category (a).

I believe you're proposing only storing category (b) headers in
include/dt-bindings/, and storing any others I suppose in arch/*/boot/dts/.

But, my thoughts are that /all/ these headers (both categories) should
be stored in one place for consistency.

That way, if/when the DT binding docs, these headers, and the DT files
themselves move out of the kernel, we'll end up with some other
repository/repositories that might have the following top-level
directories (or at least these sets of logical data):

1) DT binding documents
2) Headers that define constants for (1)
3) DT files (*.dts/*.dtsi)

We need at least some of (2) in the kernel for drivers to share the
constant definitions, so my proposal is to simply copy /all/ the headers
from (2) into the kernel's include/dt-bindings/. That keeps things
simple; simply copy everything and maintain the same hierarchy under
that "root" directory. Otherwise, we'll be constantly wondering which
headers to copy, perhaps moving things back/forth as people realize that
the kernel needs them, etc.
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Marek Feb. 21, 2013, 10:26 p.m. UTC | #3
On 20.2.2013 22:05, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> The recent dtc+cpp support allows header files and C pre-processor
> defines/macros to be used when compiling device tree files. These
> headers will typically define various constants that are part of the
> device tree bindings.
> 
> The original patch which set up the dtc+cpp include path only considered
> using those headers from device tree files. However, they are also
> useful for kernel code which needs to interpret the device tree.
> 
> In both the DT files and the kernel, I'd like to include the DT-related
> headers in the same way, for example, <dt-bindings/gpio/tegra-gpio.h>.
> That will simplify any text which discusses the DT header locations.
> 
> Creating a <dt-bindings/> for kernel source to use is as simple as
> placing files into include/dt-bindings/.
> 
> However, when compiling DT files, the include path should be restricted
> so that only the dt-bindings path is available; arbitrary kernel headers
> shouldn't be exposed. For this reason, create a specific include
> directory for use by dtc+cpp, and symlink dt-bindings from there to the
> actual location of include/dt-bindings/. For want of a better location,
> place this "include chroot" into the existing dts/ directory.

Nice trick. dts+cpp only sees the bindings and LINUXINCLUDE is not
polluted either. You can add

Acked-by: Michal Marek <mmarek@suse.cz>

if you like.

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shawn Guo Feb. 22, 2013, 2:35 a.m. UTC | #4
On Thu, Feb 21, 2013 at 11:43:13AM -0700, Stephen Warren wrote:
> There are two things that include DT-related headers:
> 
> a) Device trees (*.dts, *.dtsi)
> b) The kernel
> 
> All the headers relevant here fall into category (a) by definition. I'd
> actually expect most to also fall into category (b), although I can see
> that category (b) might be a strict subset of category (a).
> 
> I believe you're proposing only storing category (b) headers in
> include/dt-bindings/, and storing any others I suppose in arch/*/boot/dts/.
> 
> But, my thoughts are that /all/ these headers (both categories) should
> be stored in one place for consistency.
> 
> That way, if/when the DT binding docs, these headers, and the DT files
> themselves move out of the kernel, we'll end up with some other
> repository/repositories that might have the following top-level
> directories (or at least these sets of logical data):
> 
> 1) DT binding documents
> 2) Headers that define constants for (1)
> 3) DT files (*.dts/*.dtsi)
> 
> We need at least some of (2) in the kernel for drivers to share the
> constant definitions, so my proposal is to simply copy /all/ the headers
> from (2) into the kernel's include/dt-bindings/. That keeps things
> simple; simply copy everything and maintain the same hierarchy under
> that "root" directory. Otherwise, we'll be constantly wondering which
> headers to copy, perhaps moving things back/forth as people realize that
> the kernel needs them, etc.

You need to anyway identify the headers needed by a) but not b) and
remove them from linux/include/dt-bindings/, when all DTS gets moved
out of kernel tree.  Otherwise, you end up leaving those headers only
needed by DTS in the kernel tree.

Shawn

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren Feb. 22, 2013, 6:11 p.m. UTC | #5
On 02/21/2013 07:35 PM, Shawn Guo wrote:
> On Thu, Feb 21, 2013 at 11:43:13AM -0700, Stephen Warren wrote:
>> There are two things that include DT-related headers:
>>
>> a) Device trees (*.dts, *.dtsi)
>> b) The kernel
>>
>> All the headers relevant here fall into category (a) by definition. I'd
>> actually expect most to also fall into category (b), although I can see
>> that category (b) might be a strict subset of category (a).
>>
>> I believe you're proposing only storing category (b) headers in
>> include/dt-bindings/, and storing any others I suppose in arch/*/boot/dts/.
>>
>> But, my thoughts are that /all/ these headers (both categories) should
>> be stored in one place for consistency.
>>
>> That way, if/when the DT binding docs, these headers, and the DT files
>> themselves move out of the kernel, we'll end up with some other
>> repository/repositories that might have the following top-level
>> directories (or at least these sets of logical data):
>>
>> 1) DT binding documents
>> 2) Headers that define constants for (1)
>> 3) DT files (*.dts/*.dtsi)
>>
>> We need at least some of (2) in the kernel for drivers to share the
>> constant definitions, so my proposal is to simply copy /all/ the headers
>> from (2) into the kernel's include/dt-bindings/. That keeps things
>> simple; simply copy everything and maintain the same hierarchy under
>> that "root" directory. Otherwise, we'll be constantly wondering which
>> headers to copy, perhaps moving things back/forth as people realize that
>> the kernel needs them, etc.
> 
> You need to anyway identify the headers needed by a) but not b) and
> remove them from linux/include/dt-bindings/, when all DTS gets moved
> out of kernel tree.  Otherwise, you end up leaving those headers only
> needed by DTS in the kernel tree.

Well that's really my point - do you actually /need/ to do that?

What headers would be in (b) but not (a)? We'd typically be defining
headers for the purpose of defining constants that are part of the
binding definitions. To keep the kernel and binding documents in sync,
the kernel should have access to any such header. Hence, I think the set
of headers only in (a) would be non-existent. Even if there are some,
presumably there would be so few it wouldn't actually matter if we
weeded them out.

The only exception I can think of right now is the header for the i.MX
pinctrl bindings, since that's expanding the names into a set of values
the kernel will use directly rather than as needing to use as table
indices. I'm not sure if that's the right way to go on the bindings; I
see you're getting some pushback. But, I guess "pinctrl-simple" would
end up in the same boat, so perhaps there's some argument for not
removing boot/dts/ itself from the include path.
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/include/dt-bindings b/arch/arm/boot/dts/include/dt-bindings
new file mode 120000
index 0000000..08c00e4
--- /dev/null
+++ b/arch/arm/boot/dts/include/dt-bindings
@@ -0,0 +1 @@ 
+../../../../../include/dt-bindings
\ No newline at end of file
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 07125e6..d6c5036 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -157,8 +157,7 @@  cpp_flags      = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE)     \
 ld_flags       = $(LDFLAGS) $(ldflags-y)
 
 dtc_cpp_flags  = -Wp,-MD,$(depfile) -nostdinc                            \
-		 -I$(srctree)/arch/$(SRCARCH)/boot/dts                   \
-		 -I$(srctree)/arch/$(SRCARCH)/include/dts                \
+		 -I$(srctree)/arch/$(SRCARCH)/boot/dts/include           \
 		 -undef -D__DTS__
 
 # Finds the multi-part object the current object will be linked into