| Message ID | 1361394356-19385-1-git-send-email-swarren@wwwdotorg.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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.
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
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
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.
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