Re: [RFC PATCH 1/3] dtc: Add dtb build information option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



David, Rob,

On 1/17/20 3:43 PM, Rob Herring wrote:
On Fri, Jan 17, 2020 at 6:26 AM David Gibson
<david@xxxxxxxxxxxxxxxxxxxxx> wrote:

On Thu, Jan 16, 2020 at 09:58:23AM +0100, Alexandre Torgue wrote:
Hi David

On 1/16/20 1:57 AM, David Gibson wrote:
On Mon, Jan 13, 2020 at 07:16:23PM +0100, Alexandre Torgue wrote:
This commit adds the possibility to add build information for a DTB.
Build information can be: build date, DTS version, "who built the DTB"
(same kind of information that we get in Linux with the Linux banner).

To do this, an extra option "-B" using an information file as argument
has been added. If this option is used, input device tree is appended with
a new string property "Build-info". This property is built with information
found in information file given as argument. This file has to be generated
by user and shouldn't exceed 256 bytes.

Signed-off-by: Alexandre Torgue <alexandre.torgue@xxxxxx>

At the very least, this patch of the series will need to be sent to
upstream dtc first.

Ok sorry. I thought that sending all the series would give more
information.

That's fair enough, but in order to merge, you'll need to post against
upstream dtc.

ok


I'm also not terribly clear on what you're trying to accomplish here,
and why it's useful.

Let's take Kernel boot at example (but could be extend to other DTB "users"
like U-Boot). When Linux kernel booting we get a log that gives useful
information about kernel image: source version, build date, people who built
the kernel image, compiler version. This information is useful for debug and
support. The aim is to get same kind of information but for the DTB.

Since you're doing this specifically for use with dtbs built in the
kernel build, could you just use a:
     Build-info = /incbin/ "build-info.txt";
in each of the in-kernel .dts files?

My first idea was to not modify all existing .dts files. Adding an extra
option in dtc is (for me) the softer way to do it. I mean, compile
information should come through compiler without modify .dts files outside
from dtc. In this way it will be easy to everybody using dtc (inside our
outside Linux tree) to add dtb build info (even if they don't how to write a
dts file).

But you're not really having this information coming from the
compiler.  Instead you're adding a compiler option that just force
includes another file into the generated tree, and it's up to your
build scripts to put something useful into that file.

I don't really see that as preferable to modifying the .dts files.

I agree. I took example on kernel version info. It doesn't come from gcc but from auto-generated file. I thought it was the easier way to process. But I understand your concerns. As it is not generated by dtc itself, dtc should not be modified.


I also dislike the fact that the option as proposed is much more
general than the name suggests, but also very similar too, but much
more specific than the existing /incbin/ option.

What might be better would be to have a dtc option which force appends
an extra .dts to the mail .dts compiled.  You can then put an overlay
template in that file, something like:

&{/} {
         linux,build-info = /incbin/ "build-info.txt;
}

I like this suggestion either as an include another dts file or an
overlay. The latter could be useful as a way to maintain current dtb
files while splitting the source files into base and overlay dts
files.

First suggestion will imply to modify an huge part of dts file (not a big modification but a lot :)). Second one (dtbo) sounds good. In this case this dtso will be created from build-info.txt and applied when dtb is built. So no impacts on current dts file. I'm right ?

Alex


But no, let's not prepend this with 'linux'. It's not a property
specific for Linux to consume.

Rob




[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux