Hi,
The goal of this series is to add device tree build information in dtb.
This information can be dtb build date, where devicetree files come from,
who built the dtb ... Actually, same kind of information that you can find
in the Linux banner which is printout during kernel boot. Having the same
kind of information for device tree is useful for debugging and maintenance.
To achieve that a new option "-B" (using an argument) is added to dtc.
The argument is a file containing a string with build information
(e.g., From Linux 5.5.0-rc1 by alex the Mon Jan 13 18:25:38 CET 2020).
DTC use it to append dts file with a new string property "Build-info".
of/fdt.c is modified to printout "Build-info" property during Kernel boot and
scripts/Makefile.lib is modified to use dtc -B option during kernel make (this
last part could be improved for sure).
Regards
Alex
Alexandre Torgue (3):
dtc: Add dtb build information option
of: fdt: print dtb build information
scripts: Use -B dtc option to generate dtb build information.
drivers/of/fdt.c | 9 +++++++
scripts/Makefile.lib | 11 +++++---
scripts/dtc/dtc.c | 55 +++++++++++++++++++++++++++++++++-----
scripts/gen_dtb_build_info | 11 ++++++++
4 files changed, 76 insertions(+), 10 deletions(-)
create mode 100755 scripts/gen_dtb_build_info
--
2.17.1
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 <[email protected]>
diff --git a/scripts/dtc/dtc.c b/scripts/dtc/dtc.c
index bdb3f5945699..294828bac20b 100644
--- a/scripts/dtc/dtc.c
+++ b/scripts/dtc/dtc.c
@@ -18,6 +18,7 @@ int padsize; /* Additional padding to blob */
int alignsize; /* Additional padding to blob accroding to the alignsize */
int phandle_format = PHANDLE_EPAPR; /* Use linux,phandle or phandle properties */
int generate_symbols; /* enable symbols & fixup support */
+int generate_build_info; /* Add build information: time, source version ... */
int generate_fixups; /* suppress generation of fixups on symbol support */
int auto_label_aliases; /* auto generate labels -> aliases */
int annotate; /* Level of annotation: 1 for input source location
@@ -45,9 +46,42 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
fill_fullpaths(child, tree->fullpath);
}
+static void fill_build_info(struct node *tree, const char *fname)
+{
+ struct data d = empty_data;
+ char *tmp;
+ FILE *f;
+ int len;
+
+ tmp = xmalloc(sizeof(char) * 256);
+
+ f = fopen(fname, "r");
+ if (!f) {
+ printf("Can't open file %s\n", fname);
+ return;
+ }
+
+ len = fread(tmp, sizeof(char), 256, f);
+ if (!len) {
+ printf("Can't read file %s\n", fname);
+ fclose(f);
+ free(tmp);
+ }
+ fclose(f);
+
+ tmp[len - 1] = '\0';
+
+ d = data_add_marker(d, TYPE_STRING, tmp);
+ d = data_append_data(d, tmp, len);
+
+ add_property(tree, build_property("Build-info", d, NULL));
+
+ free(tmp);
+}
+
/* Usage related data. */
static const char usage_synopsis[] = "dtc [options] <input file>";
-static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AThv";
+static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AT:B:hv";
static struct option const usage_long_opts[] = {
{"quiet", no_argument, NULL, 'q'},
{"in-format", a_argument, NULL, 'I'},
@@ -69,6 +103,7 @@ static struct option const usage_long_opts[] = {
{"symbols", no_argument, NULL, '@'},
{"auto-alias", no_argument, NULL, 'A'},
{"annotate", no_argument, NULL, 'T'},
+ {"build-info", a_argument, NULL, 'B'},
{"help", no_argument, NULL, 'h'},
{"version", no_argument, NULL, 'v'},
{NULL, no_argument, NULL, 0x0},
@@ -106,6 +141,7 @@ static const char * const usage_opts_help[] = {
"\n\tEnable generation of symbols",
"\n\tEnable auto-alias of labels",
"\n\tAnnotate output .dts with input source file and line (-T -T for more details)",
+ "\n\tAdd build information (date, version, ...) in the blob",
"\n\tPrint this help and exit",
"\n\tPrint version and exit",
NULL,
@@ -164,6 +200,7 @@ int main(int argc, char *argv[])
const char *outform = NULL;
const char *outname = "-";
const char *depname = NULL;
+ const char *version = NULL;
bool force = false, sort = false;
const char *arg;
int opt;
@@ -256,9 +293,12 @@ int main(int argc, char *argv[])
case 'T':
annotate++;
break;
-
case 'h':
usage(NULL);
+ case 'B':
+ version = optarg;
+ generate_build_info = 1;
+ break;
default:
usage("unknown option");
}
@@ -296,14 +336,17 @@ int main(int argc, char *argv[])
}
if (annotate && (!streq(inform, "dts") || !streq(outform, "dts")))
die("--annotate requires -I dts -O dts\n");
- if (streq(inform, "dts"))
+ if (streq(inform, "dts")) {
dti = dt_from_source(arg);
- else if (streq(inform, "fs"))
+ if (generate_build_info)
+ fill_build_info(dti->dt, version);
+ } else if (streq(inform, "fs")) {
dti = dt_from_fs(arg);
- else if(streq(inform, "dtb"))
+ } else if (streq(inform, "dtb")) {
dti = dt_from_blob(arg);
- else
+ } else {
die("Unknown input format \"%s\"\n", inform);
+ }
dti->outname = outname;
--
2.17.1
This commit adds a new script to create a string in tmp file with
some information (date, linux version, user). This file is then used by
dtc with -B option to append dts file with a new property.
During kernel boot it will then be possible to printout DTB build
information (date, linux version used, user).
Signed-off-by: Alexandre Torgue <[email protected]>
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 3fa32f83b2d7..6a98eac1e56d 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -235,6 +235,7 @@ quiet_cmd_gzip = GZIP $@
# DTC
# ---------------------------------------------------------------------------
DTC ?= $(objtree)/scripts/dtc/dtc
+DTB_GEN_INFO ?= $(objtree)/scripts/gen_dtb_build_info
# Disable noisy checks by default
ifeq ($(findstring 1,$(KBUILD_EXTRA_WARN)),)
@@ -275,11 +276,13 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
quiet_cmd_dtc = DTC $@
cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
- $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
- $(DTC) -O $(2) -o $@ -b 0 \
+ $(DTB_GEN_INFO) $(@).info ;\
+ $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
+ $(DTC) -O $(2) -o $@ -b 0 -B $(@).info\
$(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
- -d $(depfile).dtc.tmp $(dtc-tmp) ; \
- cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
+ -d $(depfile).dtc.tmp $(dtc-tmp) ; \
+ rm $(@).info ; \
+ cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
$(call if_changed_dep,dtc,dtb)
diff --git a/scripts/gen_dtb_build_info b/scripts/gen_dtb_build_info
new file mode 100755
index 000000000000..30cf7506b9d5
--- /dev/null
+++ b/scripts/gen_dtb_build_info
@@ -0,0 +1,11 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+DTB_TARGET=$@
+COMPILE_BY=$(whoami | sed 's/\\/\\\\/')
+
+touch $DTB_TARGET
+
+{
+ echo From Linux $KERNELRELEASE by $COMPILE_BY the $(date).
+} > $DTB_TARGET
--
2.17.1
This commit prints out DTB build information (build time, dts source
version used, ...) if "Build-info" property exists in DTB root node.
Signed-off-by: Alexandre Torgue <[email protected]>
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 2cdf64d2456f..df5f54f9582a 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1224,9 +1224,18 @@ bool __init early_init_dt_scan(void *params)
*/
void __init unflatten_device_tree(void)
{
+ const char *build_info;
+ unsigned long dt_root;
+
__unflatten_device_tree(initial_boot_params, NULL, &of_root,
early_init_dt_alloc_memory_arch, false);
+ /* If available, provide dtb build information */
+ dt_root = of_get_flat_dt_root();
+ build_info = of_get_flat_dt_prop(dt_root, "Build-info", NULL);
+ if (build_info)
+ pr_info("%s\n", build_info);
+
/* Get pointer to "/chosen" and "/aliases" nodes for use everywhere */
of_alias_scan(early_init_dt_alloc_memory_arch);
--
2.17.1
Hi Alexandre!
[email protected] wrote:
>
>The goal of this series is to add device tree build information in dtb.
>This information can be dtb build date, where devicetree files come from,
>who built the dtb ... Actually, same kind of information that you can find
>in the Linux banner which is printout during kernel boot. Having the same
>kind of information for device tree is useful for debugging and maintenance.
>
>To achieve that a new option "-B" (using an argument) is added to dtc.
>The argument is a file containing a string with build information
>(e.g., From Linux 5.5.0-rc1 by alex the Mon Jan 13 18:25:38 CET 2020).
>DTC use it to append dts file with a new string property "Build-info".
>
>of/fdt.c is modified to printout "Build-info" property during Kernel boot and
>scripts/Makefile.lib is modified to use dtc -B option during kernel make (this
>last part could be improved for sure).
>
>Regards
>Alex
>
>Alexandre Torgue (3):
> dtc: Add dtb build information option
> of: fdt: print dtb build information
> scripts: Use -B dtc option to generate dtb build information.
>
> drivers/of/fdt.c | 9 +++++++
> scripts/Makefile.lib | 11 +++++---
> scripts/dtc/dtc.c | 55 +++++++++++++++++++++++++++++++++-----
> scripts/gen_dtb_build_info | 11 ++++++++
> 4 files changed, 76 insertions(+), 10 deletions(-)
> create mode 100755 scripts/gen_dtb_build_info
Looks good to me in testing here:
[ 0.000000] OF: fdt: From Linux 5.5.0-rc6-00031-g95e20af9fb9c-dirty by stemci01 the Wed 15 Jan 14:33:25 GMT 2020.
Tested-By: Steve McIntyre <[email protected]>
--
Steve McIntyre [email protected]
<http://www.linaro.org/> Linaro.org | Open source software for ARM SoCs
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 <[email protected]>
At the very least, this patch of the series will need to be sent to
upstream dtc first.
I'm also not terribly clear on what you're trying to accomplish here,
and why it's useful.
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?
Altough you probably shouldn't use "Build-info" since it doesn't match
device tree property naming conventions. My suggestion would be
"linux,build-info".
> diff --git a/scripts/dtc/dtc.c b/scripts/dtc/dtc.c
> index bdb3f5945699..294828bac20b 100644
> --- a/scripts/dtc/dtc.c
> +++ b/scripts/dtc/dtc.c
> @@ -18,6 +18,7 @@ int padsize; /* Additional padding to blob */
> int alignsize; /* Additional padding to blob accroding to the alignsize */
> int phandle_format = PHANDLE_EPAPR; /* Use linux,phandle or phandle properties */
> int generate_symbols; /* enable symbols & fixup support */
> +int generate_build_info; /* Add build information: time, source version ... */
> int generate_fixups; /* suppress generation of fixups on symbol support */
> int auto_label_aliases; /* auto generate labels -> aliases */
> int annotate; /* Level of annotation: 1 for input source location
> @@ -45,9 +46,42 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
> fill_fullpaths(child, tree->fullpath);
> }
>
> +static void fill_build_info(struct node *tree, const char *fname)
> +{
> + struct data d = empty_data;
> + char *tmp;
> + FILE *f;
> + int len;
> +
> + tmp = xmalloc(sizeof(char) * 256);
> +
> + f = fopen(fname, "r");
> + if (!f) {
> + printf("Can't open file %s\n", fname);
> + return;
> + }
> +
> + len = fread(tmp, sizeof(char), 256, f);
> + if (!len) {
> + printf("Can't read file %s\n", fname);
> + fclose(f);
> + free(tmp);
> + }
> + fclose(f);
You have no useful error reporting if the file is larger than the limit.
> +
> + tmp[len - 1] = '\0';
> +
> + d = data_add_marker(d, TYPE_STRING, tmp);
> + d = data_append_data(d, tmp, len);
You can essentially do this better with data_copy_file().
> +
> + add_property(tree, build_property("Build-info", d, NULL));
> +
> + free(tmp);
> +}
> +
> /* Usage related data. */
> static const char usage_synopsis[] = "dtc [options] <input file>";
> -static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AThv";
> +static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AT:B:hv";
> static struct option const usage_long_opts[] = {
> {"quiet", no_argument, NULL, 'q'},
> {"in-format", a_argument, NULL, 'I'},
> @@ -69,6 +103,7 @@ static struct option const usage_long_opts[] = {
> {"symbols", no_argument, NULL, '@'},
> {"auto-alias", no_argument, NULL, 'A'},
> {"annotate", no_argument, NULL, 'T'},
> + {"build-info", a_argument, NULL, 'B'},
> {"help", no_argument, NULL, 'h'},
> {"version", no_argument, NULL, 'v'},
> {NULL, no_argument, NULL, 0x0},
> @@ -106,6 +141,7 @@ static const char * const usage_opts_help[] = {
> "\n\tEnable generation of symbols",
> "\n\tEnable auto-alias of labels",
> "\n\tAnnotate output .dts with input source file and line (-T -T for more details)",
> + "\n\tAdd build information (date, version, ...) in the blob",
> "\n\tPrint this help and exit",
> "\n\tPrint version and exit",
> NULL,
> @@ -164,6 +200,7 @@ int main(int argc, char *argv[])
> const char *outform = NULL;
> const char *outname = "-";
> const char *depname = NULL;
> + const char *version = NULL;
> bool force = false, sort = false;
> const char *arg;
> int opt;
> @@ -256,9 +293,12 @@ int main(int argc, char *argv[])
> case 'T':
> annotate++;
> break;
> -
> case 'h':
> usage(NULL);
> + case 'B':
> + version = optarg;
> + generate_build_info = 1;
> + break;
> default:
> usage("unknown option");
> }
> @@ -296,14 +336,17 @@ int main(int argc, char *argv[])
> }
> if (annotate && (!streq(inform, "dts") || !streq(outform, "dts")))
> die("--annotate requires -I dts -O dts\n");
> - if (streq(inform, "dts"))
> + if (streq(inform, "dts")) {
> dti = dt_from_source(arg);
> - else if (streq(inform, "fs"))
> + if (generate_build_info)
> + fill_build_info(dti->dt, version);
> + } else if (streq(inform, "fs")) {
> dti = dt_from_fs(arg);
> - else if(streq(inform, "dtb"))
> + } else if (streq(inform, "dtb")) {
> dti = dt_from_blob(arg);
> - else
> + } else {
> die("Unknown input format \"%s\"\n", inform);
> + }
>
> dti->outname = outname;
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On 1/13/20 12:16 PM, Alexandre Torgue wrote:
> Hi,
>
> The goal of this series is to add device tree build information in dtb.
> This information can be dtb build date, where devicetree files come from,
> who built the dtb ... Actually, same kind of information that you can find
> in the Linux banner which is printout during kernel boot. Having the same
> kind of information for device tree is useful for debugging and maintenance.
>
> To achieve that a new option "-B" (using an argument) is added to dtc.
> The argument is a file containing a string with build information
> (e.g., From Linux 5.5.0-rc1 by alex the Mon Jan 13 18:25:38 CET 2020).
> DTC use it to append dts file with a new string property "Build-info".
>
> of/fdt.c is modified to printout "Build-info" property during Kernel boot and
> scripts/Makefile.lib is modified to use dtc -B option during kernel make (this
> last part could be improved for sure).
Please read through the thread at:
https://lore.kernel.org/linux-arm-kernel/[email protected]/
which was my attempt to do something similar.
-Frank
>
> Regards
> Alex
>
> Alexandre Torgue (3):
> dtc: Add dtb build information option
> of: fdt: print dtb build information
> scripts: Use -B dtc option to generate dtb build information.
>
> drivers/of/fdt.c | 9 +++++++
> scripts/Makefile.lib | 11 +++++---
> scripts/dtc/dtc.c | 55 +++++++++++++++++++++++++++++++++-----
> scripts/gen_dtb_build_info | 11 ++++++++
> 4 files changed, 76 insertions(+), 10 deletions(-)
> create mode 100755 scripts/gen_dtb_build_info
>
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 <[email protected]>
>
> 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.
>
> 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).
>
> Altough you probably shouldn't use "Build-info" since it doesn't match
> device tree property naming conventions. My suggestion would be
> "linux,build-info".
Yes I agree but something like "dtb-info" would be better as this
property can be added when dtb is built for Linux but also for U-Boot.
>
>> diff --git a/scripts/dtc/dtc.c b/scripts/dtc/dtc.c
>> index bdb3f5945699..294828bac20b 100644
>> --- a/scripts/dtc/dtc.c
>> +++ b/scripts/dtc/dtc.c
>> @@ -18,6 +18,7 @@ int padsize; /* Additional padding to blob */
>> int alignsize; /* Additional padding to blob accroding to the alignsize */
>> int phandle_format = PHANDLE_EPAPR; /* Use linux,phandle or phandle properties */
>> int generate_symbols; /* enable symbols & fixup support */
>> +int generate_build_info; /* Add build information: time, source version ... */
>> int generate_fixups; /* suppress generation of fixups on symbol support */
>> int auto_label_aliases; /* auto generate labels -> aliases */
>> int annotate; /* Level of annotation: 1 for input source location
>> @@ -45,9 +46,42 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
>> fill_fullpaths(child, tree->fullpath);
>> }
>>
>> +static void fill_build_info(struct node *tree, const char *fname)
>> +{
>> + struct data d = empty_data;
>> + char *tmp;
>> + FILE *f;
>> + int len;
>> +
>> + tmp = xmalloc(sizeof(char) * 256);
>> +
>> + f = fopen(fname, "r");
>> + if (!f) {
>> + printf("Can't open file %s\n", fname);
>> + return;
>> + }
>> +
>> + len = fread(tmp, sizeof(char), 256, f);
>> + if (!len) {
>> + printf("Can't read file %s\n", fname);
>> + fclose(f);
>> + free(tmp);
>> + }
>> + fclose(f);
>
> You have no useful error reporting if the file is larger than the limit.
Ok. To fix.
>
>> +
>> + tmp[len - 1] = '\0';
>> +
>> + d = data_add_marker(d, TYPE_STRING, tmp);
>> + d = data_append_data(d, tmp, len);
>
> You can essentially do this better with data_copy_file().
I will try with data_copy_file(). Thanks.
>
>> +
>> + add_property(tree, build_property("Build-info", d, NULL));
>> +
>> + free(tmp);
>> +}
>> +
>> /* Usage related data. */
>> static const char usage_synopsis[] = "dtc [options] <input file>";
>> -static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AThv";
>> +static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AT:B:hv";
>> static struct option const usage_long_opts[] = {
>> {"quiet", no_argument, NULL, 'q'},
>> {"in-format", a_argument, NULL, 'I'},
>> @@ -69,6 +103,7 @@ static struct option const usage_long_opts[] = {
>> {"symbols", no_argument, NULL, '@'},
>> {"auto-alias", no_argument, NULL, 'A'},
>> {"annotate", no_argument, NULL, 'T'},
>> + {"build-info", a_argument, NULL, 'B'},
>> {"help", no_argument, NULL, 'h'},
>> {"version", no_argument, NULL, 'v'},
>> {NULL, no_argument, NULL, 0x0},
>> @@ -106,6 +141,7 @@ static const char * const usage_opts_help[] = {
>> "\n\tEnable generation of symbols",
>> "\n\tEnable auto-alias of labels",
>> "\n\tAnnotate output .dts with input source file and line (-T -T for more details)",
>> + "\n\tAdd build information (date, version, ...) in the blob",
>> "\n\tPrint this help and exit",
>> "\n\tPrint version and exit",
>> NULL,
>> @@ -164,6 +200,7 @@ int main(int argc, char *argv[])
>> const char *outform = NULL;
>> const char *outname = "-";
>> const char *depname = NULL;
>> + const char *version = NULL;
>> bool force = false, sort = false;
>> const char *arg;
>> int opt;
>> @@ -256,9 +293,12 @@ int main(int argc, char *argv[])
>> case 'T':
>> annotate++;
>> break;
>> -
>> case 'h':
>> usage(NULL);
>> + case 'B':
>> + version = optarg;
>> + generate_build_info = 1;
>> + break;
>> default:
>> usage("unknown option");
>> }
>> @@ -296,14 +336,17 @@ int main(int argc, char *argv[])
>> }
>> if (annotate && (!streq(inform, "dts") || !streq(outform, "dts")))
>> die("--annotate requires -I dts -O dts\n");
>> - if (streq(inform, "dts"))
>> + if (streq(inform, "dts")) {
>> dti = dt_from_source(arg);
>> - else if (streq(inform, "fs"))
>> + if (generate_build_info)
>> + fill_build_info(dti->dt, version);
>> + } else if (streq(inform, "fs")) {
>> dti = dt_from_fs(arg);
>> - else if(streq(inform, "dtb"))
>> + } else if (streq(inform, "dtb")) {
>> dti = dt_from_blob(arg);
>> - else
>> + } else {
>> die("Unknown input format \"%s\"\n", inform);
>> + }
>>
>> dti->outname = outname;
>>
>
Hi Franck,
On 1/16/20 3:28 AM, Frank Rowand wrote:
> On 1/13/20 12:16 PM, Alexandre Torgue wrote:
>> Hi,
>>
>> The goal of this series is to add device tree build information in dtb.
>> This information can be dtb build date, where devicetree files come from,
>> who built the dtb ... Actually, same kind of information that you can find
>> in the Linux banner which is printout during kernel boot. Having the same
>> kind of information for device tree is useful for debugging and maintenance.
>>
>> To achieve that a new option "-B" (using an argument) is added to dtc.
>> The argument is a file containing a string with build information
>> (e.g., From Linux 5.5.0-rc1 by alex the Mon Jan 13 18:25:38 CET 2020).
>> DTC use it to append dts file with a new string property "Build-info".
>>
>> of/fdt.c is modified to printout "Build-info" property during Kernel boot and
>> scripts/Makefile.lib is modified to use dtc -B option during kernel make (this
>> last part could be improved for sure).
>
> Please read through the thread at:
>
> https://lore.kernel.org/linux-arm-kernel/[email protected]/
>
> which was my attempt to do something similar.
Yes the idea is the same: get build DTB information like build date,
"who built the DTB" ... The difference seems to be the way to do it. In
my case, I don't want to modify existing dts source files., but I "just"
append them by creating a new property with a string containing this
build information.
Why your proposition has not been accepted ?
Regards
Alex
>
> -Frank
>
>>
>> Regards
>> Alex
>>
>> Alexandre Torgue (3):
>> dtc: Add dtb build information option
>> of: fdt: print dtb build information
>> scripts: Use -B dtc option to generate dtb build information.
>>
>> drivers/of/fdt.c | 9 +++++++
>> scripts/Makefile.lib | 11 +++++---
>> scripts/dtc/dtc.c | 55 +++++++++++++++++++++++++++++++++-----
>> scripts/gen_dtb_build_info | 11 ++++++++
>> 4 files changed, 76 insertions(+), 10 deletions(-)
>> create mode 100755 scripts/gen_dtb_build_info
>>
>
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 <[email protected]>
> >
> > 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.
> > 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 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;
}
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On Fri, Jan 17, 2020 at 6:26 AM David Gibson
<[email protected]> 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 <[email protected]>
> > >
> > > 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.
>
> > > 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 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.
But no, let's not prepend this with 'linux'. It's not a property
specific for Linux to consume.
Rob
David, Rob,
On 1/17/20 3:43 PM, Rob Herring wrote:
> On Fri, Jan 17, 2020 at 6:26 AM David Gibson
> <[email protected]> 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 <[email protected]>
>>>>
>>>> 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
>
On 1/16/20 2:19 AM, Alexandre Torgue wrote:
> Hi Franck,
>
> On 1/16/20 3:28 AM, Frank Rowand wrote:
>> On 1/13/20 12:16 PM, Alexandre Torgue wrote:
>>> Hi,
>>>
>>> The goal of this series is to add device tree build information in dtb.
>>> This information can be dtb build date, where devicetree files come from,
>>> who built the dtb ... Actually, same kind of information that you can find
>>> in the Linux banner which is printout during kernel boot. Having the same
>>> kind of information for device tree is useful for debugging and maintenance.
>>>
>>> To achieve that a new option "-B" (using an argument) is added to dtc.
>>> The argument is a file containing a string with build information
>>> (e.g., From Linux 5.5.0-rc1 by alex the Mon Jan 13 18:25:38 CET 2020).
>>> DTC use it to append dts file with a new string property "Build-info".
>>>
>>> of/fdt.c is modified to printout "Build-info" property during Kernel boot and
>>> scripts/Makefile.lib is modified to use dtc -B option during kernel make (this
>>> last part could be improved for sure).
>>
>> Please read through the thread at:
>>
>> https://lore.kernel.org/linux-arm-kernel/[email protected]/
>>
>> which was my attempt to do something similar.
>
> Yes the idea is the same: get build DTB information like build date,
> "who built the DTB" ... The difference seems to be the way to do it.
> In my case, I don't want to modify existing dts source files., but I
> "just" append them by creating a new property with a string
> containing this build information.>
> Why your proposition has not been accepted ?
Since you are asking this question, I am presuming that you did not
read the replies in the thread I referenced. Please read through
the entire thread. Most of the review comments were objecting to
the concept of my proposal.
-Frank
>
> Regards
> Alex
>
>>
>> -Frank
>>
>>>
>>> Regards
>>> Alex
>>>
>>> Alexandre Torgue (3):
>>> dtc: Add dtb build information option
>>> of: fdt: print dtb build information
>>> scripts: Use -B dtc option to generate dtb build information.
>>>
>>> drivers/of/fdt.c | 9 +++++++
>>> scripts/Makefile.lib | 11 +++++---
>>> scripts/dtc/dtc.c | 55 +++++++++++++++++++++++++++++++++-----
>>> scripts/gen_dtb_build_info | 11 ++++++++
>>> 4 files changed, 76 insertions(+), 10 deletions(-)
>>> create mode 100755 scripts/gen_dtb_build_info
>>>
>>
>
On 1/13/20 12:16 PM, Alexandre Torgue wrote:
> This commit adds a new script to create a string in tmp file with
> some information (date, linux version, user). This file is then used by
> dtc with -B option to append dts file with a new property.
> During kernel boot it will then be possible to printout DTB build
> information (date, linux version used, user).
>
> Signed-off-by: Alexandre Torgue <[email protected]>
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 3fa32f83b2d7..6a98eac1e56d 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -235,6 +235,7 @@ quiet_cmd_gzip = GZIP $@
> # DTC
> # ---------------------------------------------------------------------------
> DTC ?= $(objtree)/scripts/dtc/dtc
> +DTB_GEN_INFO ?= $(objtree)/scripts/gen_dtb_build_info
>
> # Disable noisy checks by default
> ifeq ($(findstring 1,$(KBUILD_EXTRA_WARN)),)
> @@ -275,11 +276,13 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>
> quiet_cmd_dtc = DTC $@
> cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
> - $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
> - $(DTC) -O $(2) -o $@ -b 0 \
> + $(DTB_GEN_INFO) $(@).info ;\
> + $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
> + $(DTC) -O $(2) -o $@ -b 0 -B $(@).info\
> $(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
> - -d $(depfile).dtc.tmp $(dtc-tmp) ; \
> - cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
> + -d $(depfile).dtc.tmp $(dtc-tmp) ; \
> + rm $(@).info ; \
> + cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
>
> $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
> $(call if_changed_dep,dtc,dtb)
> diff --git a/scripts/gen_dtb_build_info b/scripts/gen_dtb_build_info
> new file mode 100755
> index 000000000000..30cf7506b9d5
> --- /dev/null
> +++ b/scripts/gen_dtb_build_info
> @@ -0,0 +1,11 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +
> +DTB_TARGET=$@
> +COMPILE_BY=$(whoami | sed 's/\\/\\\\/')
> +
> +touch $DTB_TARGET
> +
> +{
> + echo From Linux $KERNELRELEASE by $COMPILE_BY the $(date).
> +} > $DTB_TARGET
>
This specific set of information does not seem to me to be sufficient
to be of much use. In my previous attempt to capture build time
information into the DTB I included more information that this,
which I felt provided more of the information that would be valuable
to a developer (or testing person) in a development environment,
test environment, or on an end user system. The exact set of
information is easy to bike shed over, but one could explain what
information might be useful and why (I did not provide that explanation
in my patch series, but in retrospect should have).
On Fri, Jan 17, 2020 at 04:11:19PM +0100, Alexandre Torgue wrote:
> David, Rob,
>
> On 1/17/20 3:43 PM, Rob Herring wrote:
> > On Fri, Jan 17, 2020 at 6:26 AM David Gibson
> > <[email protected]> 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 <[email protected]>
> > > > >
> > > > > 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 :)).
I'm not exactly sure what you're meaning by the "first suggestion" here.
> 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 ?
This is not a dtbo, it's using the compile time overlaying syntax.
.dtbo would be useless for this purpose, since the build information
would be detached from the built dtb.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On Fri, Jan 17, 2020 at 08:43:23AM -0600, Rob Herring wrote:
> On Fri, Jan 17, 2020 at 6:26 AM David Gibson
> <[email protected]> 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 <[email protected]>
> > > >
> > > > 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.
> >
> > > > 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 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.
Sorry, to be clear what I'm talking about here is just including
another dts file, and using the compile-type overlay syntax. This is
not the same as .dtbo style runtime overlays (though the final result
is about the same in this case).
> The latter could be useful as a way to maintain current dtb
> files while splitting the source files into base and overlay dts
> files.
>
> But no, let's not prepend this with 'linux'. It's not a property
> specific for Linux to consume.
It's not really about who consumes it. It's about defining a
namespace for the new property to exist in, since it's not part of a
relevant standard (if we wanted to make it such, we should pin down
what goes in there with much more precision).
This is specific to files built in the Linux tree, hence my suggestion
of "linux", whoever ends up consuming them.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Hi Franck
On 1/17/20 8:13 PM, Frank Rowand wrote:
>
> On 1/16/20 2:19 AM, Alexandre Torgue wrote:
>> Hi Franck,
>>
>> On 1/16/20 3:28 AM, Frank Rowand wrote:
>>> On 1/13/20 12:16 PM, Alexandre Torgue wrote:
>>>> Hi,
>>>>
>>>> The goal of this series is to add device tree build information in dtb.
>>>> This information can be dtb build date, where devicetree files come from,
>>>> who built the dtb ... Actually, same kind of information that you can find
>>>> in the Linux banner which is printout during kernel boot. Having the same
>>>> kind of information for device tree is useful for debugging and maintenance.
>>>>
>>>> To achieve that a new option "-B" (using an argument) is added to dtc.
>>>> The argument is a file containing a string with build information
>>>> (e.g., From Linux 5.5.0-rc1 by alex the Mon Jan 13 18:25:38 CET 2020).
>>>> DTC use it to append dts file with a new string property "Build-info".
>>>>
>>>> of/fdt.c is modified to printout "Build-info" property during Kernel boot and
>>>> scripts/Makefile.lib is modified to use dtc -B option during kernel make (this
>>>> last part could be improved for sure).
>>>
>>> Please read through the thread at:
>>>
>>> https://lore.kernel.org/linux-arm-kernel/[email protected]/
>>>
>>> which was my attempt to do something similar.
>>
>> Yes the idea is the same: get build DTB information like build date,
>> "who built the DTB" ... The difference seems to be the way to do it.
>> In my case, I don't want to modify existing dts source files., but I
>> "just" append them by creating a new property with a string
>> containing this build information.>
>> Why your proposition has not been accepted ?
>
> Since you are asking this question, I am presuming that you did not
> read the replies in the thread I referenced. Please read through
> the entire thread. Most of the review comments were objecting to
> the concept of my proposal.
Sorry I have been lazy :$. This series is a tiny one compare to yours,
and adds less dtb information (only git used, who built dtb and the
date). There are no "dtb versions", and "absolute/relative" path which
created concerns. One remaining concern is "reproducible build" but
making dtb info optional could respond to it.
I continue to think that it's useful to get those information (even if
it's only a string) But before continuing (and find a better technical
way to do it), I need to know if there are no other obstacles.
Regards
Alex
>
> -Frank
>
>>
>> Regards
>> Alex
>>
>>>
>>> -Frank
>>>
>>>>
>>>> Regards
>>>> Alex
>>>>
>>>> Alexandre Torgue (3):
>>>> dtc: Add dtb build information option
>>>> of: fdt: print dtb build information
>>>> scripts: Use -B dtc option to generate dtb build information.
>>>>
>>>> drivers/of/fdt.c | 9 +++++++
>>>> scripts/Makefile.lib | 11 +++++---
>>>> scripts/dtc/dtc.c | 55 +++++++++++++++++++++++++++++++++-----
>>>> scripts/gen_dtb_build_info | 11 ++++++++
>>>> 4 files changed, 76 insertions(+), 10 deletions(-)
>>>> create mode 100755 scripts/gen_dtb_build_info
>>>>
>>>
>>
>
On 1/20/20 4:56 AM, Alexandre Torgue wrote:
> Hi Franck
>
> On 1/17/20 8:13 PM, Frank Rowand wrote:
>>
>> On 1/16/20 2:19 AM, Alexandre Torgue wrote:
>>> Hi Franck,
>>>
>>> On 1/16/20 3:28 AM, Frank Rowand wrote:
>>>> On 1/13/20 12:16 PM, Alexandre Torgue wrote:
>>>>> Hi,
>>>>>
>>>>> The goal of this series is to add device tree build information in dtb.
>>>>> This information can be dtb build date, where devicetree files come from,
>>>>> who built the dtb ... Actually, same kind of information that you can find
>>>>> in the Linux banner which is printout during kernel boot. Having the same
>>>>> kind of information for device tree is useful for debugging and maintenance.
>>>>>
>>>>> To achieve that a new option "-B" (using an argument) is added to dtc.
>>>>> The argument is a file containing a string with build information
>>>>> (e.g., From Linux 5.5.0-rc1 by alex the Mon Jan 13 18:25:38 CET 2020).
>>>>> DTC use it to append dts file with a new string property "Build-info".
>>>>>
>>>>> of/fdt.c is modified to printout "Build-info" property during Kernel boot and
>>>>> scripts/Makefile.lib is modified to use dtc -B option during kernel make (this
>>>>> last part could be improved for sure).
>>>>
>>>> Please read through the thread at:
>>>>
>>>> https://lore.kernel.org/linux-arm-kernel/[email protected]/
>>>>
>>>> which was my attempt to do something similar.
>>>
>>> Yes the idea is the same: get build DTB information like build date,
>>> "who built the DTB" ... The difference seems to be the way to do it.
>>> In my case, I don't want to modify existing dts source files., but I
>>> "just" append them by creating a new property with a string
>>> containing this build information.>
>>> Why your proposition has not been accepted ?
>>
>> Since you are asking this question, I am presuming that you did not
>> read the replies in the thread I referenced. Please read through
>> the entire thread. Most of the review comments were objecting to
>> the concept of my proposal.
>
> Sorry I have been lazy :$. This series is a tiny one compare to
> yours, and adds less dtb information (only git used, who built dtb
> and the date). There are no "dtb versions", and "absolute/relative"
> path which created concerns. One remaining concern is "reproducible
Here is an example of the info from one of my builds:
From Linux 5.5.0-rc2-dirty by frowand the Mon Jan 20 09:50:58 CST 2020.
The information 'Linux 5.5.0-rc2-dirty' is precisely what was most objected
to in my proposal.
-Frank
> build" but making dtb info optional could respond to it. I continue
> to think that it's useful to get those information (even if it's only
> a string) But before continuing (and find a better technical way to
> do it), I need to know if there are no other obstacles.
>
> Regards
> Alex
>
>
>>
>> -Frank
>>
>>>
>>> Regards
>>> Alex
>>>
>>>>
>>>> -Frank
>>>>
>>>>>
>>>>> Regards
>>>>> Alex
>>>>>
>>>>> Alexandre Torgue (3):
>>>>> dtc: Add dtb build information option
>>>>> of: fdt: print dtb build information
>>>>> scripts: Use -B dtc option to generate dtb build information.
>>>>>
>>>>> drivers/of/fdt.c | 9 +++++++
>>>>> scripts/Makefile.lib | 11 +++++---
>>>>> scripts/dtc/dtc.c | 55 +++++++++++++++++++++++++++++++++-----
>>>>> scripts/gen_dtb_build_info | 11 ++++++++
>>>>> 4 files changed, 76 insertions(+), 10 deletions(-)
>>>>> create mode 100755 scripts/gen_dtb_build_info
>>>>>
>>>>
>>>
>>
>
On 1/13/20 12:16 PM, Alexandre Torgue wrote:
> This commit adds a new script to create a string in tmp file with
> some information (date, linux version, user). This file is then used by
> dtc with -B option to append dts file with a new property.
> During kernel boot it will then be possible to printout DTB build
> information (date, linux version used, user).
>
> Signed-off-by: Alexandre Torgue <[email protected]>
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 3fa32f83b2d7..6a98eac1e56d 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -235,6 +235,7 @@ quiet_cmd_gzip = GZIP $@
> # DTC
> # ---------------------------------------------------------------------------
> DTC ?= $(objtree)/scripts/dtc/dtc
> +DTB_GEN_INFO ?= $(objtree)/scripts/gen_dtb_build_info
>
> # Disable noisy checks by default
> ifeq ($(findstring 1,$(KBUILD_EXTRA_WARN)),)
> @@ -275,11 +276,13 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>
> quiet_cmd_dtc = DTC $@
> cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
> - $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
> - $(DTC) -O $(2) -o $@ -b 0 \
> + $(DTB_GEN_INFO) $(@).info ;\
> + $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
> + $(DTC) -O $(2) -o $@ -b 0 -B $(@).info\
> $(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
> - -d $(depfile).dtc.tmp $(dtc-tmp) ; \
> - cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
> + -d $(depfile).dtc.tmp $(dtc-tmp) ; \
> + rm $(@).info ; \
> + cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
The indentation should be tabs instead of spaces.
-Frank
>
> $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
> $(call if_changed_dep,dtc,dtb)
> diff --git a/scripts/gen_dtb_build_info b/scripts/gen_dtb_build_info
> new file mode 100755
> index 000000000000..30cf7506b9d5
> --- /dev/null
> +++ b/scripts/gen_dtb_build_info
> @@ -0,0 +1,11 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +
> +DTB_TARGET=$@
> +COMPILE_BY=$(whoami | sed 's/\\/\\\\/')
> +
> +touch $DTB_TARGET
> +
> +{
> + echo From Linux $KERNELRELEASE by $COMPILE_BY the $(date).
> +} > $DTB_TARGET
>
On Fri, Jan 17, 2020 at 08:43:23AM -0600, Rob Herring wrote:
>On Fri, Jan 17, 2020 at 6:26 AM David Gibson
><[email protected]> wrote:
...
>> 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.
ACK, that sounds like it could be helpful.
>But no, let's not prepend this with 'linux'. It's not a property
>specific for Linux to consume.
Right. We might be seeing the data coming through from U-Boot (or any
other random bootloader) too.
Cheers,
--
Steve McIntyre [email protected]
<http://www.linaro.org/> Linaro.org | Open source software for ARM SoCs
Hi Frank!
Thanks for the link back to the previous discussion, it's very
helpful.
On Mon, Jan 20, 2020 at 10:14:22AM -0600, Frank Rowand wrote:
>On 1/20/20 4:56 AM, Alexandre Torgue wrote:
...
>> and the date). There are no "dtb versions", and "absolute/relative"
>> path which created concerns. One remaining concern is "reproducible
>
>Here is an example of the info from one of my builds:
>
> From Linux 5.5.0-rc2-dirty by frowand the Mon Jan 20 09:50:58 CST 2020.
>
>The information 'Linux 5.5.0-rc2-dirty' is precisely what was most objected
>to in my proposal.
ACK. :-( I'm surprised to see so much push-back on what looks like a
simple piece of information here.
I've had users *specifically* asking for this kind of identification
so that they can verify the version of the DTB they're using at
runtime. Right now it can be a guessing game, which does not help
people trying to debug problems.
Cheers,
--
Steve McIntyre [email protected]
<http://www.linaro.org/> Linaro.org | Open source software for ARM SoCs
On 1/20/20 12:28 PM, Steve McIntyre wrote:
> Hi Frank!
>
> Thanks for the link back to the previous discussion, it's very
> helpful.
>
> On Mon, Jan 20, 2020 at 10:14:22AM -0600, Frank Rowand wrote:
>> On 1/20/20 4:56 AM, Alexandre Torgue wrote:
>
> ...
>
>>> and the date). There are no "dtb versions", and "absolute/relative"
>>> path which created concerns. One remaining concern is "reproducible
>>
>> Here is an example of the info from one of my builds:
>>
>> From Linux 5.5.0-rc2-dirty by frowand the Mon Jan 20 09:50:58 CST 2020.
>>
>> The information 'Linux 5.5.0-rc2-dirty' is precisely what was most objected
>> to in my proposal.
>
> ACK. :-( I'm surprised to see so much push-back on what looks like a
> simple piece of information here.
Me too.
>
> I've had users *specifically* asking for this kind of identification
> so that they can verify the version of the DTB they're using at
> runtime. Right now it can be a guessing game, which does not help
> people trying to debug problems.
>
> Cheers,
>
If the information was reported as debug information via pr_debug(),
would that work for your use case? Or would the users' kernels
not have debug enabled in the configuration?
-Frank
On 1/20/20 9:20 PM, Frank Rowand wrote:
> On 1/20/20 12:28 PM, Steve McIntyre wrote:
>> Hi Frank!
>>
>> Thanks for the link back to the previous discussion, it's very
>> helpful.
>>
>> On Mon, Jan 20, 2020 at 10:14:22AM -0600, Frank Rowand wrote:
>>> On 1/20/20 4:56 AM, Alexandre Torgue wrote:
>>
>> ...
>>
>>>> and the date). There are no "dtb versions", and "absolute/relative"
>>>> path which created concerns. One remaining concern is "reproducible
>>>
>>> Here is an example of the info from one of my builds:
>>>
>>> From Linux 5.5.0-rc2-dirty by frowand the Mon Jan 20 09:50:58 CST 2020.
>>>
>>> The information 'Linux 5.5.0-rc2-dirty' is precisely what was most objected
>>> to in my proposal.
>>
>> ACK. :-( I'm surprised to see so much push-back on what looks like a
>> simple piece of information here.
>
> Me too.
>
>
>>
>> I've had users *specifically* asking for this kind of identification
>> so that they can verify the version of the DTB they're using at
>> runtime. Right now it can be a guessing game, which does not help
>> people trying to debug problems.
>>
>> Cheers,
>>
>
> If the information was reported as debug information via pr_debug(),
> would that work for your use case? Or would the users' kernels
> not have debug enabled in the configuration?
And even pr_debug() might not be sufficient since the property
value is available via /proc/device-tree if the proc file
system is enabled.
On Sun, Jan 19, 2020 at 12:41 AM David Gibson
<[email protected]> wrote:
>
> On Fri, Jan 17, 2020 at 08:43:23AM -0600, Rob Herring wrote:
> > On Fri, Jan 17, 2020 at 6:26 AM David Gibson
> > <[email protected]> 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 <[email protected]>
> > > > >
> > > > > 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.
> > >
> > > > > 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 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.
>
> Sorry, to be clear what I'm talking about here is just including
> another dts file, and using the compile-type overlay syntax. This is
> not the same as .dtbo style runtime overlays (though the final result
> is about the same in this case).
Ah, okay. That's probably easier to implement.
> > The latter could be useful as a way to maintain current dtb
> > files while splitting the source files into base and overlay dts
> > files.
> >
> > But no, let's not prepend this with 'linux'. It's not a property
> > specific for Linux to consume.
>
> It's not really about who consumes it. It's about defining a
> namespace for the new property to exist in, since it's not part of a
> relevant standard (if we wanted to make it such, we should pin down
> what goes in there with much more precision).
I can't think of any cases of the 'linux' prefix not being about who
consumes it. And we often end up dropping 'linux' because it turns out
to not be Linux specific. I don't care to see u-boot,build-info,
freebsd,build-info, etc. when a given dtb can only have 1 of those.
My intent is this property name is added to the DT spec, but I don't
agree we should define what's in it beyond a string. It is information
that is useful for humans identifying what the dtb was built from.
Rob
[ Adding lakml to the CC list ]
On Mon, Jan 20, 2020 at 09:20:55PM -0600, Frank Rowand wrote:
>On 1/20/20 12:28 PM, Steve McIntyre wrote:
>> On Mon, Jan 20, 2020 at 10:14:22AM -0600, Frank Rowand wrote:
>>> On 1/20/20 4:56 AM, Alexandre Torgue wrote:
>>>
>>> Here is an example of the info from one of my builds:
>>>
>>> From Linux 5.5.0-rc2-dirty by frowand the Mon Jan 20 09:50:58 CST 2020.
>>>
>>> The information 'Linux 5.5.0-rc2-dirty' is precisely what was most objected
>>> to in my proposal.
>>
>> ACK. :-( I'm surprised to see so much push-back on what looks like a
>> simple piece of information here.
>
>Me too.
So, looking at the comments back on the old thread...
Alexandre is proposing somthing slightly different here: a patch to
add a simple string to allow for a description of where the DTB came
from. The particular example he uses here fills in build details from
the Linux repo, but it could just as easily be filled in as part of a
U-Boot build, or the build of a DTB included with EDK2, or whatever
other firmware might include it. It might be useful to also add
similar debug output into U-Boot, or for that matter any other
DT-using project.
As Rob says later, it's simply information for humans to help identify
where a DTB came from. Nothing more.
>> I've had users *specifically* asking for this kind of identification
>> so that they can verify the version of the DTB they're using at
>> runtime. Right now it can be a guessing game, which does not help
>> people trying to debug problems.
>
>If the information was reported as debug information via pr_debug(),
>would that work for your use case? Or would the users' kernels
>not have debug enabled in the configuration?
Quite possibly not - I'm not 100% sure to be honest. :-/
--
Steve McIntyre [email protected]
<http://www.linaro.org/> Linaro.org | Open source software for ARM SoCs
On Tue, Jan 21, 2020 at 09:59:44AM -0600, Rob Herring wrote:
>On Sun, Jan 19, 2020 at 12:41 AM David Gibson
><[email protected]> wrote:
>>
>> It's not really about who consumes it. It's about defining a
>> namespace for the new property to exist in, since it's not part of a
>> relevant standard (if we wanted to make it such, we should pin down
>> what goes in there with much more precision).
>
>I can't think of any cases of the 'linux' prefix not being about who
>consumes it. And we often end up dropping 'linux' because it turns out
>to not be Linux specific. I don't care to see u-boot,build-info,
>freebsd,build-info, etc. when a given dtb can only have 1 of those.
Yes, exactly. What would happen if somebody (tried to) fill in more
than one of XXXX.build-info? It makes no sense.
>My intent is this property name is added to the DT spec, but I don't
>agree we should define what's in it beyond a string. It is information
>that is useful for humans identifying what the dtb was built from.
Nod - defining this as a free-form string lets people put their own
information in, without us having to try and agree on a full spec
which we'll need to update as ideas change.
--
Steve McIntyre [email protected]
<http://www.linaro.org/> Linaro.org | Open source software for ARM SoCs
Hi
On 1/20/20 7:17 PM, Steve McIntyre wrote:
> On Fri, Jan 17, 2020 at 08:43:23AM -0600, Rob Herring wrote:
>> On Fri, Jan 17, 2020 at 6:26 AM David Gibson
>> <[email protected]> wrote:
>
> ...
>
>>> 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.
>
> ACK, that sounds like it could be helpful.
>
>> But no, let's not prepend this with 'linux'. It's not a property
>> specific for Linux to consume.
>
> Right. We might be seeing the data coming through from U-Boot (or any
> other random bootloader) too.
>
> Cheers,
>
Thanks for reviews. I gonna prepare a V2 with David proposition (to use
overlay format) by keeping in mind not to modify existing dts(i) files.
Remaining questions are:
1- "build-info" or "linux,build-info"? IMO, If information is "generic"
then first one should be used.
2- Looking at Franck proposition[1] some years ago and objections on it,
do you think that this one could accepted ?
regards
Alex
[1] https://lore.kernel.org/linux-arm-kernel/[email protected]/
On 1/17/20 1:20 PM, Frank Rowand wrote:
> On 1/13/20 12:16 PM, Alexandre Torgue wrote:
>> This commit adds a new script to create a string in tmp file with
>> some information (date, linux version, user). This file is then used by
>> dtc with -B option to append dts file with a new property.
>> During kernel boot it will then be possible to printout DTB build
>> information (date, linux version used, user).
>>
>> Signed-off-by: Alexandre Torgue <[email protected]>
>>
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index 3fa32f83b2d7..6a98eac1e56d 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -235,6 +235,7 @@ quiet_cmd_gzip = GZIP $@
>> # DTC
>> # ---------------------------------------------------------------------------
>> DTC ?= $(objtree)/scripts/dtc/dtc
>> +DTB_GEN_INFO ?= $(objtree)/scripts/gen_dtb_build_info
>>
>> # Disable noisy checks by default
>> ifeq ($(findstring 1,$(KBUILD_EXTRA_WARN)),)
>> @@ -275,11 +276,13 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>>
>> quiet_cmd_dtc = DTC $@
>> cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
>> - $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
>> - $(DTC) -O $(2) -o $@ -b 0 \
>> + $(DTB_GEN_INFO) $(@).info ;\
>> + $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
>> + $(DTC) -O $(2) -o $@ -b 0 -B $(@).info\
>> $(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
>> - -d $(depfile).dtc.tmp $(dtc-tmp) ; \
>> - cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
>> + -d $(depfile).dtc.tmp $(dtc-tmp) ; \
>> + rm $(@).info ; \
>> + cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
>>
>> $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
>> $(call if_changed_dep,dtc,dtb)
>> diff --git a/scripts/gen_dtb_build_info b/scripts/gen_dtb_build_info
>> new file mode 100755
>> index 000000000000..30cf7506b9d5
>> --- /dev/null
>> +++ b/scripts/gen_dtb_build_info
>> @@ -0,0 +1,11 @@
>> +#!/bin/sh
>> +# SPDX-License-Identifier: GPL-2.0
>> +
>> +DTB_TARGET=$@
>> +COMPILE_BY=$(whoami | sed 's/\\/\\\\/')
>> +
>> +touch $DTB_TARGET
>> +
>> +{
>> + echo From Linux $KERNELRELEASE by $COMPILE_BY the $(date).
A nit, the trailing period is not needed. Not a big deal one way
or the other.
>> +} > $DTB_TARGET
>>
>
> This specific set of information does not seem to me to be sufficient
> to be of much use. In my previous attempt to capture build time
> information into the DTB I included more information that this,
> which I felt provided more of the information that would be valuable
> to a developer (or testing person) in a development environment,
> test environment, or on an end user system. The exact set of
> information is easy to bike shed over, but one could explain what
> information might be useful and why (I did not provide that explanation
> in my patch series, but in retrospect should have).
On reflection, this information is sufficient. My concern was that a
unique version number was not provided. But the unique version number
_is_ provided by the date, which is hh:mm:ss, so sufficient if the
dtb is not compiled more often than once per second. So good enough
for the debugging environment.
On 1/22/20 12:00 PM, Alexandre Torgue wrote:
> Hi
>
> On 1/20/20 7:17 PM, Steve McIntyre wrote:
>> On Fri, Jan 17, 2020 at 08:43:23AM -0600, Rob Herring wrote:
>>> On Fri, Jan 17, 2020 at 6:26 AM David Gibson
>>> <[email protected]> wrote:
>>
>> ...
>>
>>>> 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.
>>
>> ACK, that sounds like it could be helpful.
>>
>>> But no, let's not prepend this with 'linux'. It's not a property
>>> specific for Linux to consume.
>>
>> Right. We might be seeing the data coming through from U-Boot (or any
>> other random bootloader) too.
>>
>> Cheers,
>>
>
> Thanks for reviews. I gonna prepare a V2 with David proposition (to use overlay format) by keeping in mind not to modify existing dts(i) files.
>
> Remaining questions are:
>
> 1- "build-info" or "linux,build-info"? IMO, If information is
> "generic" then first one should be used.
I would prefer build-info. The data may be generated by a non-linux
build environment, such as uboot.
> 2- Looking at Franck proposition[1] some years ago and objections on
> it, do you think that this one could accepted ?
I think that with the few small changes suggested in this thread, that
the old objections are not relevant to your version.
>
> regards
> Alex
>
> [1] https://lore.kernel.org/linux-arm-kernel/[email protected]/
>
>
>
>
>
On Tue, Jan 21, 2020 at 09:59:44AM -0600, Rob Herring wrote:
> On Sun, Jan 19, 2020 at 12:41 AM David Gibson
> <[email protected]> wrote:
> >
> > On Fri, Jan 17, 2020 at 08:43:23AM -0600, Rob Herring wrote:
> > > On Fri, Jan 17, 2020 at 6:26 AM David Gibson
> > > <[email protected]> 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 <[email protected]>
> > > > > >
> > > > > > 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.
> > > >
> > > > > > 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 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.
> >
> > Sorry, to be clear what I'm talking about here is just including
> > another dts file, and using the compile-type overlay syntax. This is
> > not the same as .dtbo style runtime overlays (though the final result
> > is about the same in this case).
>
> Ah, okay. That's probably easier to implement.
>
> > > The latter could be useful as a way to maintain current dtb
> > > files while splitting the source files into base and overlay dts
> > > files.
> > >
> > > But no, let's not prepend this with 'linux'. It's not a property
> > > specific for Linux to consume.
> >
> > It's not really about who consumes it. It's about defining a
> > namespace for the new property to exist in, since it's not part of a
> > relevant standard (if we wanted to make it such, we should pin down
> > what goes in there with much more precision).
>
> I can't think of any cases of the 'linux' prefix not being about who
> consumes it. And we often end up dropping 'linux' because it turns out
> to not be Linux specific. I don't care to see u-boot,build-info,
> freebsd,build-info, etc. when a given dtb can only have 1 of those.
But all other vendor prefixes are about who generated or specified the
information, not who consumes it, e.g. "ibm,XXX", "fsl,YYY", etc.
> My intent is this property name is added to the DT spec, but I don't
> agree we should define what's in it beyond a string. It is information
> that is useful for humans identifying what the dtb was built from.
>
> Rob
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On Wed, Jan 22, 2020 at 11:13 PM David Gibson
<[email protected]> wrote:
>
> On Tue, Jan 21, 2020 at 09:59:44AM -0600, Rob Herring wrote:
> > On Sun, Jan 19, 2020 at 12:41 AM David Gibson
> > <[email protected]> wrote:
> > >
> > > On Fri, Jan 17, 2020 at 08:43:23AM -0600, Rob Herring wrote:
> > > > On Fri, Jan 17, 2020 at 6:26 AM David Gibson
> > > > <[email protected]> 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 <[email protected]>
> > > > > > >
> > > > > > > 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.
> > > > >
> > > > > > > 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 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.
> > >
> > > Sorry, to be clear what I'm talking about here is just including
> > > another dts file, and using the compile-type overlay syntax. This is
> > > not the same as .dtbo style runtime overlays (though the final result
> > > is about the same in this case).
> >
> > Ah, okay. That's probably easier to implement.
> >
> > > > The latter could be useful as a way to maintain current dtb
> > > > files while splitting the source files into base and overlay dts
> > > > files.
> > > >
> > > > But no, let's not prepend this with 'linux'. It's not a property
> > > > specific for Linux to consume.
> > >
> > > It's not really about who consumes it. It's about defining a
> > > namespace for the new property to exist in, since it's not part of a
> > > relevant standard (if we wanted to make it such, we should pin down
> > > what goes in there with much more precision).
> >
> > I can't think of any cases of the 'linux' prefix not being about who
> > consumes it. And we often end up dropping 'linux' because it turns out
> > to not be Linux specific. I don't care to see u-boot,build-info,
> > freebsd,build-info, etc. when a given dtb can only have 1 of those.
>
> But all other vendor prefixes are about who generated or specified the
> information, not who consumes it, e.g. "ibm,XXX", "fsl,YYY", etc.
I'd say those are both typically. Those are consumed by IBM and FSL
specific drivers.
But I think the better argument is what Frank said. If the
firmware/bootloader provides the dtb that it built, we'd still want
the information printed.
Rob