diff mbox series

dtc: add ability to make nodes conditional on them being referenced

Message ID 20200121102317.82798-1-andre.przywara@arm.com
State New
Headers show
Series dtc: add ability to make nodes conditional on them being referenced | expand

Commit Message

Andre Przywara Jan. 21, 2020, 10:23 a.m. UTC
From: Maxime Ripard <maxime.ripard at bootlin.com>

This is needed when importing mainline DTs into U-Boot, as some started
using this /omit-if-no-ref/ tag, so won't compile with U-Boot's current
dtc copy. This is just a cherry-pick of the patch introducing this
feature.
Original commit message from Maxime:
------------------
A number of platforms have a need to reduce the number of DT nodes,
mostly because of two similar constraints: the size of the DT blob, and
the time it takes to parse it.

As the DT is used in more and more SoCs, and by more projects, some
constraints start to appear in bootloaders running from SRAM with an
order of magnitude of 10kB. A typical DT is in the same order of
magnitude, so any effort to reduce the blob size is welcome in such an
environment.

Some platforms also want to reach very fast boot time, and the time it
takes to parse a typical DT starts to be noticeable.

Both of these issues can be mitigated by reducing the number of nodes in
the DT. The biggest provider of nodes is usually the pin controller and
its subnodes, usually one for each valid pin configuration in a given
SoC.

Obviously, a single, fixed, set of these nodes will be used by a given
board, so we can introduce a node property that will tell the DT
compiler to drop the nodes when they are not referenced in the tree, and
as such wouldn't be useful in the targetted system.

Signed-off-by: Maxime Ripard <maxime.ripard at bootlin.com>
Reviewed-by: Rob Herring <robh at kernel.org>
Signed-off-by: Andre Przywara <andre.przywara at arm.com>
---
 scripts/dtc/checks.c     | 13 +++++++++++++
 scripts/dtc/dtc-lexer.l  |  7 +++++++
 scripts/dtc/dtc-parser.y | 17 +++++++++++++++++
 scripts/dtc/dtc.h        |  4 ++++
 scripts/dtc/livetree.c   | 14 ++++++++++++++
 5 files changed, 55 insertions(+)

Comments

Tom Rini Jan. 25, 2020, 4:20 p.m. UTC | #1
On Tue, Jan 21, 2020 at 10:23:17AM +0000, Andre Przywara wrote:

> From: Maxime Ripard <maxime.ripard at bootlin.com>
> 
> This is needed when importing mainline DTs into U-Boot, as some started
> using this /omit-if-no-ref/ tag, so won't compile with U-Boot's current
> dtc copy. This is just a cherry-pick of the patch introducing this
> feature.
> Original commit message from Maxime:
> ------------------
> A number of platforms have a need to reduce the number of DT nodes,
> mostly because of two similar constraints: the size of the DT blob, and
> the time it takes to parse it.
> 
> As the DT is used in more and more SoCs, and by more projects, some
> constraints start to appear in bootloaders running from SRAM with an
> order of magnitude of 10kB. A typical DT is in the same order of
> magnitude, so any effort to reduce the blob size is welcome in such an
> environment.
> 
> Some platforms also want to reach very fast boot time, and the time it
> takes to parse a typical DT starts to be noticeable.
> 
> Both of these issues can be mitigated by reducing the number of nodes in
> the DT. The biggest provider of nodes is usually the pin controller and
> its subnodes, usually one for each valid pin configuration in a given
> SoC.
> 
> Obviously, a single, fixed, set of these nodes will be used by a given
> board, so we can introduce a node property that will tell the DT
> compiler to drop the nodes when they are not referenced in the tree, and
> as such wouldn't be useful in the targetted system.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard at bootlin.com>
> Reviewed-by: Rob Herring <robh at kernel.org>
> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> ---
>  scripts/dtc/checks.c     | 13 +++++++++++++
>  scripts/dtc/dtc-lexer.l  |  7 +++++++
>  scripts/dtc/dtc-parser.y | 17 +++++++++++++++++
>  scripts/dtc/dtc.h        |  4 ++++
>  scripts/dtc/livetree.c   | 14 ++++++++++++++
>  5 files changed, 55 insertions(+)
> 
> diff --git a/scripts/dtc/checks.c b/scripts/dtc/checks.c
> index c07ba4da9e..40879677c8 100644
> --- a/scripts/dtc/checks.c
> +++ b/scripts/dtc/checks.c
> @@ -579,6 +579,8 @@ static void fixup_phandle_references(struct check *c, struct dt_info *dti,
>  
>  			phandle = get_node_phandle(dt, refnode);
>  			*((fdt32_t *)(prop->val.val + m->offset)) = cpu_to_fdt32(phandle);
> +
> +			reference_node(refnode);
>  		}
>  	}
>  }
> @@ -609,11 +611,21 @@ static void fixup_path_references(struct check *c, struct dt_info *dti,
>  			path = refnode->fullpath;
>  			prop->val = data_insert_at_marker(prop->val, m, path,
>  							  strlen(path) + 1);
> +
> +			reference_node(refnode);
>  		}
>  	}
>  }
>  ERROR(path_references, fixup_path_references, NULL, &duplicate_node_names);
>  
> +static void fixup_omit_unused_nodes(struct check *c, struct dt_info *dti,
> +				    struct node *node)
> +{
> +	if (node->omit_if_unused && !node->is_referenced)
> +		delete_node(node);
> +}
> +ERROR(omit_unused_nodes, fixup_omit_unused_nodes, NULL, &phandle_references, &path_references);
> +
>  /*
>   * Semantic checks
>   */
> @@ -1367,6 +1379,7 @@ static struct check *check_table[] = {
>  
>  	&explicit_phandles,
>  	&phandle_references, &path_references,
> +	&omit_unused_nodes,
>  
>  	&address_cells_is_cell, &size_cells_is_cell, &interrupt_cells_is_cell,
>  	&device_type_is_string, &model_is_string, &status_is_string,
> diff --git a/scripts/dtc/dtc-lexer.l b/scripts/dtc/dtc-lexer.l
> index fd825ebba6..615b7ec658 100644
> --- a/scripts/dtc/dtc-lexer.l
> +++ b/scripts/dtc/dtc-lexer.l
> @@ -153,6 +153,13 @@ static void PRINTF(1, 2) lexical_error(const char *fmt, ...);
>  			return DT_DEL_NODE;
>  		}
>  
> +<*>"/omit-if-no-ref/"	{
> +			DPRINT("Keyword: /omit-if-no-ref/\n");
> +			DPRINT("<PROPNODENAME>\n");
> +			BEGIN(PROPNODENAME);
> +			return DT_OMIT_NO_REF;
> +		}
> +
>  <*>{LABEL}:	{
>  			DPRINT("Label: %s\n", yytext);
>  			yylval.labelref = xstrdup(yytext);
> diff --git a/scripts/dtc/dtc-parser.y b/scripts/dtc/dtc-parser.y
> index 44af170abf..66ff7f7d8e 100644
> --- a/scripts/dtc/dtc-parser.y
> +++ b/scripts/dtc/dtc-parser.y
> @@ -63,6 +63,7 @@ extern bool treesource_error;
>  %token DT_BITS
>  %token DT_DEL_PROP
>  %token DT_DEL_NODE
> +%token DT_OMIT_NO_REF
>  %token <propnodename> DT_PROPNODENAME
>  %token <integer> DT_LITERAL
>  %token <integer> DT_CHAR_LITERAL
> @@ -217,6 +218,18 @@ devicetree:
>  				ERROR(&@3, "Label or path %s not found", $3);
>  
>  
> +			$$ = $1;
> +		}
> +	| devicetree DT_OMIT_NO_REF DT_REF ';'
> +		{
> +			struct node *target = get_node_by_ref($1, $3);
> +
> +			if (target)
> +				omit_node_if_unused(target);
> +			else
> +				ERROR(&@3, "Label or path %s not found", $3);
> +
> +
>  			$$ = $1;
>  		}
>  	;
> @@ -523,6 +536,10 @@ subnode:
>  		{
>  			$$ = name_node(build_node_delete(), $2);
>  		}
> +	| DT_OMIT_NO_REF subnode
> +		{
> +			$$ = omit_node_if_unused($2);
> +		}
>  	| DT_LABEL subnode
>  		{
>  			add_label(&$2->labels, $1);
> diff --git a/scripts/dtc/dtc.h b/scripts/dtc/dtc.h
> index 3b18a42b86..6d667701ab 100644
> --- a/scripts/dtc/dtc.h
> +++ b/scripts/dtc/dtc.h
> @@ -168,6 +168,8 @@ struct node {
>  
>  	struct label *labels;
>  	const struct bus_type *bus;
> +
> +	bool omit_if_unused, is_referenced;
>  };
>  
>  #define for_each_label_withdel(l0, l) \
> @@ -202,6 +204,8 @@ struct property *reverse_properties(struct property *first);
>  struct node *build_node(struct property *proplist, struct node *children);
>  struct node *build_node_delete(void);
>  struct node *name_node(struct node *node, char *name);
> +struct node *omit_node_if_unused(struct node *node);
> +struct node *reference_node(struct node *node);
>  struct node *chain_node(struct node *first, struct node *list);
>  struct node *merge_nodes(struct node *old_node, struct node *new_node);
>  struct node *add_orphan_node(struct node *old_node, struct node *new_node, char *ref);
> diff --git a/scripts/dtc/livetree.c b/scripts/dtc/livetree.c
> index 57b7db2ed1..81b6c48454 100644
> --- a/scripts/dtc/livetree.c
> +++ b/scripts/dtc/livetree.c
> @@ -134,6 +134,20 @@ struct node *name_node(struct node *node, char *name)
>  	return node;
>  }
>  
> +struct node *omit_node_if_unused(struct node *node)
> +{
> +	node->omit_if_unused = 1;
> +
> +	return node;
> +}
> +
> +struct node *reference_node(struct node *node)
> +{
> +	node->is_referenced = 1;
> +
> +	return node;
> +}
> +
>  struct node *merge_nodes(struct node *old_node, struct node *new_node)
>  {
>  	struct property *new_prop, *old_prop;

OK, what does this flag _do_ ?  I've not been able to make it discard
anything in some tests where I scatter it all over dts files.  Is there
some other flag we also need to pass to dtc to have this be used?  I
would like to explore this option to help with platforms like tbs2910
(rather than if we can the patch Anatolij did) if we can.  Thanks!
Andre Przywara Jan. 25, 2020, 10:18 p.m. UTC | #2
On 25/01/2020 16:20, Tom Rini wrote:

Hi Tom,

thanks for having a look!


> On Tue, Jan 21, 2020 at 10:23:17AM +0000, Andre Przywara wrote:
> 
>> From: Maxime Ripard <maxime.ripard at bootlin.com>
>>
>> This is needed when importing mainline DTs into U-Boot, as some started
>> using this /omit-if-no-ref/ tag, so won't compile with U-Boot's current
>> dtc copy. This is just a cherry-pick of the patch introducing this
>> feature.
>> Original commit message from Maxime:
>> ------------------
>> A number of platforms have a need to reduce the number of DT nodes,
>> mostly because of two similar constraints: the size of the DT blob, and
>> the time it takes to parse it.
>>
>> As the DT is used in more and more SoCs, and by more projects, some
>> constraints start to appear in bootloaders running from SRAM with an
>> order of magnitude of 10kB. A typical DT is in the same order of
>> magnitude, so any effort to reduce the blob size is welcome in such an
>> environment.
>>
>> Some platforms also want to reach very fast boot time, and the time it
>> takes to parse a typical DT starts to be noticeable.
>>
>> Both of these issues can be mitigated by reducing the number of nodes in
>> the DT. The biggest provider of nodes is usually the pin controller and
>> its subnodes, usually one for each valid pin configuration in a given
>> SoC.
>>
>> Obviously, a single, fixed, set of these nodes will be used by a given
>> board, so we can introduce a node property that will tell the DT
>> compiler to drop the nodes when they are not referenced in the tree, and
>> as such wouldn't be useful in the targetted system.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard at bootlin.com>
>> Reviewed-by: Rob Herring <robh at kernel.org>
>> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
>> ---
>>  scripts/dtc/checks.c     | 13 +++++++++++++
>>  scripts/dtc/dtc-lexer.l  |  7 +++++++
>>  scripts/dtc/dtc-parser.y | 17 +++++++++++++++++
>>  scripts/dtc/dtc.h        |  4 ++++
>>  scripts/dtc/livetree.c   | 14 ++++++++++++++
>>  5 files changed, 55 insertions(+)
>>
>> diff --git a/scripts/dtc/checks.c b/scripts/dtc/checks.c
>> index c07ba4da9e..40879677c8 100644
>> --- a/scripts/dtc/checks.c
>> +++ b/scripts/dtc/checks.c
>> @@ -579,6 +579,8 @@ static void fixup_phandle_references(struct check *c, struct dt_info *dti,
>>  
>>  			phandle = get_node_phandle(dt, refnode);
>>  			*((fdt32_t *)(prop->val.val + m->offset)) = cpu_to_fdt32(phandle);
>> +
>> +			reference_node(refnode);
>>  		}
>>  	}
>>  }
>> @@ -609,11 +611,21 @@ static void fixup_path_references(struct check *c, struct dt_info *dti,
>>  			path = refnode->fullpath;
>>  			prop->val = data_insert_at_marker(prop->val, m, path,
>>  							  strlen(path) + 1);
>> +
>> +			reference_node(refnode);
>>  		}
>>  	}
>>  }
>>  ERROR(path_references, fixup_path_references, NULL, &duplicate_node_names);
>>  
>> +static void fixup_omit_unused_nodes(struct check *c, struct dt_info *dti,
>> +				    struct node *node)
>> +{
>> +	if (node->omit_if_unused && !node->is_referenced)
>> +		delete_node(node);
>> +}
>> +ERROR(omit_unused_nodes, fixup_omit_unused_nodes, NULL, &phandle_references, &path_references);
>> +
>>  /*
>>   * Semantic checks
>>   */
>> @@ -1367,6 +1379,7 @@ static struct check *check_table[] = {
>>  
>>  	&explicit_phandles,
>>  	&phandle_references, &path_references,
>> +	&omit_unused_nodes,
>>  
>>  	&address_cells_is_cell, &size_cells_is_cell, &interrupt_cells_is_cell,
>>  	&device_type_is_string, &model_is_string, &status_is_string,
>> diff --git a/scripts/dtc/dtc-lexer.l b/scripts/dtc/dtc-lexer.l
>> index fd825ebba6..615b7ec658 100644
>> --- a/scripts/dtc/dtc-lexer.l
>> +++ b/scripts/dtc/dtc-lexer.l
>> @@ -153,6 +153,13 @@ static void PRINTF(1, 2) lexical_error(const char *fmt, ...);
>>  			return DT_DEL_NODE;
>>  		}
>>  
>> +<*>"/omit-if-no-ref/"	{
>> +			DPRINT("Keyword: /omit-if-no-ref/\n");
>> +			DPRINT("<PROPNODENAME>\n");
>> +			BEGIN(PROPNODENAME);
>> +			return DT_OMIT_NO_REF;
>> +		}
>> +
>>  <*>{LABEL}:	{
>>  			DPRINT("Label: %s\n", yytext);
>>  			yylval.labelref = xstrdup(yytext);
>> diff --git a/scripts/dtc/dtc-parser.y b/scripts/dtc/dtc-parser.y
>> index 44af170abf..66ff7f7d8e 100644
>> --- a/scripts/dtc/dtc-parser.y
>> +++ b/scripts/dtc/dtc-parser.y
>> @@ -63,6 +63,7 @@ extern bool treesource_error;
>>  %token DT_BITS
>>  %token DT_DEL_PROP
>>  %token DT_DEL_NODE
>> +%token DT_OMIT_NO_REF
>>  %token <propnodename> DT_PROPNODENAME
>>  %token <integer> DT_LITERAL
>>  %token <integer> DT_CHAR_LITERAL
>> @@ -217,6 +218,18 @@ devicetree:
>>  				ERROR(&@3, "Label or path %s not found", $3);
>>  
>>  
>> +			$$ = $1;
>> +		}
>> +	| devicetree DT_OMIT_NO_REF DT_REF ';'
>> +		{
>> +			struct node *target = get_node_by_ref($1, $3);
>> +
>> +			if (target)
>> +				omit_node_if_unused(target);
>> +			else
>> +				ERROR(&@3, "Label or path %s not found", $3);
>> +
>> +
>>  			$$ = $1;
>>  		}
>>  	;
>> @@ -523,6 +536,10 @@ subnode:
>>  		{
>>  			$$ = name_node(build_node_delete(), $2);
>>  		}
>> +	| DT_OMIT_NO_REF subnode
>> +		{
>> +			$$ = omit_node_if_unused($2);
>> +		}
>>  	| DT_LABEL subnode
>>  		{
>>  			add_label(&$2->labels, $1);
>> diff --git a/scripts/dtc/dtc.h b/scripts/dtc/dtc.h
>> index 3b18a42b86..6d667701ab 100644
>> --- a/scripts/dtc/dtc.h
>> +++ b/scripts/dtc/dtc.h
>> @@ -168,6 +168,8 @@ struct node {
>>  
>>  	struct label *labels;
>>  	const struct bus_type *bus;
>> +
>> +	bool omit_if_unused, is_referenced;
>>  };
>>  
>>  #define for_each_label_withdel(l0, l) \
>> @@ -202,6 +204,8 @@ struct property *reverse_properties(struct property *first);
>>  struct node *build_node(struct property *proplist, struct node *children);
>>  struct node *build_node_delete(void);
>>  struct node *name_node(struct node *node, char *name);
>> +struct node *omit_node_if_unused(struct node *node);
>> +struct node *reference_node(struct node *node);
>>  struct node *chain_node(struct node *first, struct node *list);
>>  struct node *merge_nodes(struct node *old_node, struct node *new_node);
>>  struct node *add_orphan_node(struct node *old_node, struct node *new_node, char *ref);
>> diff --git a/scripts/dtc/livetree.c b/scripts/dtc/livetree.c
>> index 57b7db2ed1..81b6c48454 100644
>> --- a/scripts/dtc/livetree.c
>> +++ b/scripts/dtc/livetree.c
>> @@ -134,6 +134,20 @@ struct node *name_node(struct node *node, char *name)
>>  	return node;
>>  }
>>  
>> +struct node *omit_node_if_unused(struct node *node)
>> +{
>> +	node->omit_if_unused = 1;
>> +
>> +	return node;
>> +}
>> +
>> +struct node *reference_node(struct node *node)
>> +{
>> +	node->is_referenced = 1;
>> +
>> +	return node;
>> +}
>> +
>>  struct node *merge_nodes(struct node *old_node, struct node *new_node)
>>  {
>>  	struct property *new_prop, *old_prop;
> 
> OK, what does this flag _do_ ?  I've not been able to make it discard
> anything in some tests where I scatter it all over dts files.

The idea is to omit *marked* nodes that are *not referenced* by other
nodes. This is meant for pinmux nodes and old style clocks. To be
useful, it would be *prefixing* those nodes in .dtsi files, so that you
can describe all possible pins in the SoC file, for instance, but only
those actually used by your board are build into the .dtb.
Check
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sun7i-a20.dtsi#n756
for an example.

Looking at this case for instance I was actually wondering if we should
have something that applies to all child nodes. Also I was experimenting
with removing nodes that have 'status = "disabled";', but not sure that
helps you in your case.

> Is there
> some other flag we also need to pass to dtc to have this be used?

If you build with symbols generation (-@), no nodes are removed, as they
might be later used by overlays. Otherwise it's is effect without any
extra switch.

Worked for me in this example:

/dts-v1/;
/ {
        /omit-if-no-ref/
        node1: node1 {
                compatible = "foo";
                value = <17>;
        };

        /omit-if-no-ref/
        node2: node2 {
                compatible = "bar";
                value = <37>;
        };

        main {
                compatible = "baz";
                requires = <&node1>;
        };
};

$ dtc -o test.dtb test.dts && dtc test.dtb

Cheers,
Andre.
Simon Glass Jan. 28, 2020, 5:07 p.m. UTC | #3
Hi Andre,

On Sat, 25 Jan 2020 at 16:18, André Przywara <andre.przywara at arm.com> wrote:
>
> On 25/01/2020 16:20, Tom Rini wrote:
>
> Hi Tom,
>
> thanks for having a look!
>
>
> > On Tue, Jan 21, 2020 at 10:23:17AM +0000, Andre Przywara wrote:
> >
> >> From: Maxime Ripard <maxime.ripard at bootlin.com>
> >>
> >> This is needed when importing mainline DTs into U-Boot, as some started
> >> using this /omit-if-no-ref/ tag, so won't compile with U-Boot's current
> >> dtc copy. This is just a cherry-pick of the patch introducing this
> >> feature.
> >> Original commit message from Maxime:
> >> ------------------
> >> A number of platforms have a need to reduce the number of DT nodes,
> >> mostly because of two similar constraints: the size of the DT blob, and
> >> the time it takes to parse it.
> >>
> >> As the DT is used in more and more SoCs, and by more projects, some
> >> constraints start to appear in bootloaders running from SRAM with an
> >> order of magnitude of 10kB. A typical DT is in the same order of
> >> magnitude, so any effort to reduce the blob size is welcome in such an
> >> environment.
> >>
> >> Some platforms also want to reach very fast boot time, and the time it
> >> takes to parse a typical DT starts to be noticeable.
> >>
> >> Both of these issues can be mitigated by reducing the number of nodes in
> >> the DT. The biggest provider of nodes is usually the pin controller and
> >> its subnodes, usually one for each valid pin configuration in a given
> >> SoC.
> >>
> >> Obviously, a single, fixed, set of these nodes will be used by a given
> >> board, so we can introduce a node property that will tell the DT
> >> compiler to drop the nodes when they are not referenced in the tree, and
> >> as such wouldn't be useful in the targetted system.
> >>
> >> Signed-off-by: Maxime Ripard <maxime.ripard at bootlin.com>
> >> Reviewed-by: Rob Herring <robh at kernel.org>
> >> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> >> ---
> >>  scripts/dtc/checks.c     | 13 +++++++++++++
> >>  scripts/dtc/dtc-lexer.l  |  7 +++++++
> >>  scripts/dtc/dtc-parser.y | 17 +++++++++++++++++
> >>  scripts/dtc/dtc.h        |  4 ++++
> >>  scripts/dtc/livetree.c   | 14 ++++++++++++++
> >>  5 files changed, 55 insertions(+)

Reviewed-by: Simon Glass <sjg at chromium.org>

Is this for SPL or U-Boot proper? I assume the former since you talk
about SRAM. But changing dtc won't affect SPL at present, since the DT
is not separately compiled for SPL.  Of course if nodes are not needed
for U-Boot proper, removing them is good and may have SPL too. But we
use dtoc to drop unwanted nodes (as you probably know), and U-Boot has
its own tags for indicating what nodes should be present (since
everything is omitted from SPL by default).

Regards,
Simon
Andre Przywara Jan. 28, 2020, 5:29 p.m. UTC | #4
On Tue, 28 Jan 2020 10:07:18 -0700
Simon Glass <sjg at chromium.org> wrote:

Hi Simon,

> On Sat, 25 Jan 2020 at 16:18, André Przywara <andre.przywara at arm.com> wrote:
> >
> > On 25/01/2020 16:20, Tom Rini wrote:
> >
> > Hi Tom,
> >
> > thanks for having a look!
> >
> >  
> > > On Tue, Jan 21, 2020 at 10:23:17AM +0000, Andre Przywara wrote:
> > >  
> > >> From: Maxime Ripard <maxime.ripard at bootlin.com>
> > >>
> > >> This is needed when importing mainline DTs into U-Boot, as some started
> > >> using this /omit-if-no-ref/ tag, so won't compile with U-Boot's current
> > >> dtc copy. This is just a cherry-pick of the patch introducing this
> > >> feature.
> > >> Original commit message from Maxime:
> > >> ------------------
> > >> A number of platforms have a need to reduce the number of DT nodes,
> > >> mostly because of two similar constraints: the size of the DT blob, and
> > >> the time it takes to parse it.
> > >>
> > >> As the DT is used in more and more SoCs, and by more projects, some
> > >> constraints start to appear in bootloaders running from SRAM with an
> > >> order of magnitude of 10kB. A typical DT is in the same order of
> > >> magnitude, so any effort to reduce the blob size is welcome in such an
> > >> environment.
> > >>
> > >> Some platforms also want to reach very fast boot time, and the time it
> > >> takes to parse a typical DT starts to be noticeable.
> > >>
> > >> Both of these issues can be mitigated by reducing the number of nodes in
> > >> the DT. The biggest provider of nodes is usually the pin controller and
> > >> its subnodes, usually one for each valid pin configuration in a given
> > >> SoC.
> > >>
> > >> Obviously, a single, fixed, set of these nodes will be used by a given
> > >> board, so we can introduce a node property that will tell the DT
> > >> compiler to drop the nodes when they are not referenced in the tree, and
> > >> as such wouldn't be useful in the targetted system.
> > >>
> > >> Signed-off-by: Maxime Ripard <maxime.ripard at bootlin.com>
> > >> Reviewed-by: Rob Herring <robh at kernel.org>
> > >> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> > >> ---
> > >>  scripts/dtc/checks.c     | 13 +++++++++++++
> > >>  scripts/dtc/dtc-lexer.l  |  7 +++++++
> > >>  scripts/dtc/dtc-parser.y | 17 +++++++++++++++++
> > >>  scripts/dtc/dtc.h        |  4 ++++
> > >>  scripts/dtc/livetree.c   | 14 ++++++++++++++
> > >>  5 files changed, 55 insertions(+)  
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>

Thanks!

> 
> Is this for SPL or U-Boot proper?

I don't think either of those was the original target here, as this showed up in the Linux tree first.

If you need an answer to your question: it would actually be U-Boot proper, as it originates from Maxime's work on some Allwinner A20 devices - and we don't use DT in the SPL on sunxi.

> I assume the former since you talk about SRAM.

I think the idea was to provide a generic solution for some specific problem. The observation was that the .dtb is growing with all those pinmux nodes, also reportedly boot time increased because of the parsing efforts.
So short of hacking/trimming the DT manually, this tag was introduced, to address the issue in a generic way.

> But changing dtc won't affect SPL at present, since the DT
> is not separately compiled for SPL.  Of course if nodes are not needed
> for U-Boot proper, removing them is good and may have SPL too. But we
> use dtoc to drop unwanted nodes (as you probably know), and U-Boot has
> its own tags for indicating what nodes should be present (since
> everything is omitted from SPL by default).

Understood. As mentioned, sunxi doesn't even use the DT at all in the SPL.

The reason I posted this patch is simply that some mainline Linux .dts files carry this tag now, and without U-Boot's dtc understanding it, those DTs fail to build.
So for the sake of copying .dts files verbatim from the kernel tree, we need dtc support for this tag.

Cheers,
Andre.
diff mbox series

Patch

diff --git a/scripts/dtc/checks.c b/scripts/dtc/checks.c
index c07ba4da9e..40879677c8 100644
--- a/scripts/dtc/checks.c
+++ b/scripts/dtc/checks.c
@@ -579,6 +579,8 @@  static void fixup_phandle_references(struct check *c, struct dt_info *dti,
 
 			phandle = get_node_phandle(dt, refnode);
 			*((fdt32_t *)(prop->val.val + m->offset)) = cpu_to_fdt32(phandle);
+
+			reference_node(refnode);
 		}
 	}
 }
@@ -609,11 +611,21 @@  static void fixup_path_references(struct check *c, struct dt_info *dti,
 			path = refnode->fullpath;
 			prop->val = data_insert_at_marker(prop->val, m, path,
 							  strlen(path) + 1);
+
+			reference_node(refnode);
 		}
 	}
 }
 ERROR(path_references, fixup_path_references, NULL, &duplicate_node_names);
 
+static void fixup_omit_unused_nodes(struct check *c, struct dt_info *dti,
+				    struct node *node)
+{
+	if (node->omit_if_unused && !node->is_referenced)
+		delete_node(node);
+}
+ERROR(omit_unused_nodes, fixup_omit_unused_nodes, NULL, &phandle_references, &path_references);
+
 /*
  * Semantic checks
  */
@@ -1367,6 +1379,7 @@  static struct check *check_table[] = {
 
 	&explicit_phandles,
 	&phandle_references, &path_references,
+	&omit_unused_nodes,
 
 	&address_cells_is_cell, &size_cells_is_cell, &interrupt_cells_is_cell,
 	&device_type_is_string, &model_is_string, &status_is_string,
diff --git a/scripts/dtc/dtc-lexer.l b/scripts/dtc/dtc-lexer.l
index fd825ebba6..615b7ec658 100644
--- a/scripts/dtc/dtc-lexer.l
+++ b/scripts/dtc/dtc-lexer.l
@@ -153,6 +153,13 @@  static void PRINTF(1, 2) lexical_error(const char *fmt, ...);
 			return DT_DEL_NODE;
 		}
 
+<*>"/omit-if-no-ref/"	{
+			DPRINT("Keyword: /omit-if-no-ref/\n");
+			DPRINT("<PROPNODENAME>\n");
+			BEGIN(PROPNODENAME);
+			return DT_OMIT_NO_REF;
+		}
+
 <*>{LABEL}:	{
 			DPRINT("Label: %s\n", yytext);
 			yylval.labelref = xstrdup(yytext);
diff --git a/scripts/dtc/dtc-parser.y b/scripts/dtc/dtc-parser.y
index 44af170abf..66ff7f7d8e 100644
--- a/scripts/dtc/dtc-parser.y
+++ b/scripts/dtc/dtc-parser.y
@@ -63,6 +63,7 @@  extern bool treesource_error;
 %token DT_BITS
 %token DT_DEL_PROP
 %token DT_DEL_NODE
+%token DT_OMIT_NO_REF
 %token <propnodename> DT_PROPNODENAME
 %token <integer> DT_LITERAL
 %token <integer> DT_CHAR_LITERAL
@@ -217,6 +218,18 @@  devicetree:
 				ERROR(&@3, "Label or path %s not found", $3);
 
 
+			$$ = $1;
+		}
+	| devicetree DT_OMIT_NO_REF DT_REF ';'
+		{
+			struct node *target = get_node_by_ref($1, $3);
+
+			if (target)
+				omit_node_if_unused(target);
+			else
+				ERROR(&@3, "Label or path %s not found", $3);
+
+
 			$$ = $1;
 		}
 	;
@@ -523,6 +536,10 @@  subnode:
 		{
 			$$ = name_node(build_node_delete(), $2);
 		}
+	| DT_OMIT_NO_REF subnode
+		{
+			$$ = omit_node_if_unused($2);
+		}
 	| DT_LABEL subnode
 		{
 			add_label(&$2->labels, $1);
diff --git a/scripts/dtc/dtc.h b/scripts/dtc/dtc.h
index 3b18a42b86..6d667701ab 100644
--- a/scripts/dtc/dtc.h
+++ b/scripts/dtc/dtc.h
@@ -168,6 +168,8 @@  struct node {
 
 	struct label *labels;
 	const struct bus_type *bus;
+
+	bool omit_if_unused, is_referenced;
 };
 
 #define for_each_label_withdel(l0, l) \
@@ -202,6 +204,8 @@  struct property *reverse_properties(struct property *first);
 struct node *build_node(struct property *proplist, struct node *children);
 struct node *build_node_delete(void);
 struct node *name_node(struct node *node, char *name);
+struct node *omit_node_if_unused(struct node *node);
+struct node *reference_node(struct node *node);
 struct node *chain_node(struct node *first, struct node *list);
 struct node *merge_nodes(struct node *old_node, struct node *new_node);
 struct node *add_orphan_node(struct node *old_node, struct node *new_node, char *ref);
diff --git a/scripts/dtc/livetree.c b/scripts/dtc/livetree.c
index 57b7db2ed1..81b6c48454 100644
--- a/scripts/dtc/livetree.c
+++ b/scripts/dtc/livetree.c
@@ -134,6 +134,20 @@  struct node *name_node(struct node *node, char *name)
 	return node;
 }
 
+struct node *omit_node_if_unused(struct node *node)
+{
+	node->omit_if_unused = 1;
+
+	return node;
+}
+
+struct node *reference_node(struct node *node)
+{
+	node->is_referenced = 1;
+
+	return node;
+}
+
 struct node *merge_nodes(struct node *old_node, struct node *new_node)
 {
 	struct property *new_prop, *old_prop;