On Fri, Jan 10, 2014 at 7:30 AM, Rob Herring <robherring2@xxxxxxxxx> wrote: > On Fri, Jan 10, 2014 at 7:28 AM, Tomasz Figa <t.figa@xxxxxxxxxxx> wrote: >> Hi, >> >> On 10.01.2014 03:41, Olof Johansson wrote: >>> >>> On Thu, Jan 9, 2014 at 6:41 PM, Shawn Guo <shawn.guo@xxxxxxxxxx> wrote: >>>> >>>> On Sat, Jan 04, 2014 at 09:10:58AM +0800, Shawn Guo wrote: >>>>> >>>>> On Fri, Jan 03, 2014 at 11:29:35AM -0800, Olof Johansson wrote: >>>>>> >>>>>> On Thu, Jan 2, 2014 at 7:04 PM, Shawn Guo <shawn.guo@xxxxxxxxxx> wrote: >>>>>>> >>>>>>> On Thu, Jan 02, 2014 at 06:41:30PM -0800, Olof Johansson wrote: >>>>>>>> >>>>>>>> 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. > >> I'm not officially a DT maintainer, but let me share my thoughts on this. > > We can fix that... :) > >> 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
