DT include files

On Fri, Jan 10, 2014 at 7:30 AM, Rob Herring [off-list ref] wrote:

On Fri, Jan 10, 2014 at 7:28 AM, Tomasz Figa [off-list ref] wrote:

quoted

Hi,

On 10.01.2014 03:41, Olof Johansson wrote:

quoted

On Thu, Jan 9, 2014 at 6:41 PM, Shawn Guo [off-list ref] wrote:

quoted

On Sat, Jan 04, 2014 at 09:10:58AM +0800, Shawn Guo wrote:

quoted

On Fri, Jan 03, 2014 at 11:29:35AM -0800, Olof Johansson wrote:

quoted

On Thu, Jan 2, 2014 at 7:04 PM, Shawn Guo [off-list ref] wrote:

quoted

On Thu, Jan 02, 2014 at 06:41:30PM -0800, Olof Johansson wrote:

quoted

Ok, then it's probably just the location of the header files that
should be adjusted. Other subsystems have placed them under
include/dt-bindings/<subsystem>, so that's likely a better place for
these as well, don't you think?

I had a little discussion with DT people when the headers were firstly
created.  These pinctrl headers are a little different from the
headers
in include/dt-bindings/<subsystem>.  The latter are used by both
kernel
and device tree sources, while the pinctrl headers are used by device
tree sources only, so I chose to put them just in the same folder as
dts files.  And DT people are fine with my take.

Since you don't provide references to this I had to go searching for
it. All I find is some discussion from 8 months ago, and quite a bit
of that seems to have been about changing bindings, and some about the
preprocessor behavior. Also, the patches seem to have been too big to
make it out on the lists.

I'd like a fresh look from DT people on this just to make sure no
opinions have changed -- lots of things have changed in the last 8
months w.r.t. DT.

Indeed, it's been quite a long time.  Let me restate my point.  The
include/dt-bindings is introduced as a folder to hold headers that are
referenced by both kernel and DTS.  That's why we create the folder in
the kernel include folder and have arch/arm/boot/dts/include/dt-bindings
being a symbol link to it.  All the headers in there need to be
duplicated between kernel and DTS tree, when we move DTS files into
a separated repository.  Putting DTS local headers into the folder is
absolutely unnecessary, and will only confuse people and bother
ourselves when moving DTS files out of kernel tree.

Just a gentle ping to ensure we do not get the pull request lost.  Or do
you have any further comment?

Still waiting on DT maintainers to chime in.

It helps (slightly) to be copied directly.

I should have copied you on some of the thread, but this was also
right around when your calxeda address was going away so I might have
missed you.

quoted

I'm not officially a DT maintainer, but let me share my thoughts on this.

We can fix that... :)

quoted

So what options we have for this:

 1) include/dt-bindings - this directory is designed to contain headers
    that define the ABI between firmware and kernel code, in other
    words - DT bindings.

 2) arch/*/boot/dts - we already have files that can be included by
    other files in this directory, i.e. *.dtsi. Some are used to
    implement hierarchies of devices (e.g. s3c64xx.dtsi), but some are
    purely used as headers (s3c64xx-pinctrl.dtsi).

 3) include/dts? - I'm not sure if this have any benefits, but I'm
    listing it since it's one of the ideas that came to my mind.

Now 2) seems to be already used and doesn't seem to generate any problems,
so I'm all for it. Still, there is one more issue. For files to be included
merely by DTS files and so limited by DTS+CPP syntax, I don't think it's too
good idea to call them *.h. Let's stay with *.dtsi, since that's the file
extension supposed to be used for files that can be included from device
tree sources.

I think having 1 and 2 is the right way. Minimizing the number of
shared headers between dts and the kernel is a good thing.

As for *.h vs. *.dtsi naming, if the include is only pre-processor
defines, then I think using .h is the right way. Otherwise, if there
is any dts syntax, then .dtsi is the right name. It looks to me like
this style has been followed in both the imx and s3c64xx cases.

I'm mostly reacting to the use of .h for something that isn't C
preprocessing. Since dtc/dts has their own namespace for everything
else, it seems appropriate to have something unique. But this is
pretty far up bikeshed alley.

On a side note, I'm not really a fan of this pattern:

#define FOO1 1 2 3

#define BAR FOO1 FOO2 FOO3

While it definitely simplifies the dts files, it would never be used
in C and obfuscates what the actual property size is. Reading a dts
file, I would naturally assume the define was a single value while in
fact it could expand to a very large property size.

Hmm. Shawn?


-Olof