All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dtc: add ability to make nodes conditional on them being referenced
@ 2020-01-21 10:23 Andre Przywara
  2020-01-25 16:20 ` Tom Rini
  0 siblings, 1 reply; 12+ messages in thread
From: Andre Przywara @ 2020-01-21 10:23 UTC (permalink / raw)
  To: u-boot

From: Maxime Ripard <maxime.ripard@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@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Andre Przywara <andre.przywara@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;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] dtc: add ability to make nodes conditional on them being referenced
  2020-01-21 10:23 [PATCH] dtc: add ability to make nodes conditional on them being referenced Andre Przywara
@ 2020-01-25 16:20 ` Tom Rini
  2020-01-25 22:18   ` André Przywara
  0 siblings, 1 reply; 12+ messages in thread
From: Tom Rini @ 2020-01-25 16:20 UTC (permalink / raw)
  To: u-boot

On Tue, Jan 21, 2020 at 10:23:17AM +0000, Andre Przywara wrote:

> From: Maxime Ripard <maxime.ripard@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@bootlin.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Andre Przywara <andre.przywara@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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200125/f588f671/attachment.sig>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] dtc: add ability to make nodes conditional on them being referenced
  2020-01-25 16:20 ` Tom Rini
@ 2020-01-25 22:18   ` André Przywara
  2020-01-28 17:07     ` Simon Glass
  0 siblings, 1 reply; 12+ messages in thread
From: André Przywara @ 2020-01-25 22:18 UTC (permalink / raw)
  To: u-boot

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@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@bootlin.com>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Andre Przywara <andre.przywara@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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] dtc: add ability to make nodes conditional on them being referenced
  2020-01-25 22:18   ` André Przywara
@ 2020-01-28 17:07     ` Simon Glass
  2020-01-28 17:29       ` Andre Przywara
  2020-02-05 17:57       ` sjg at google.com
  0 siblings, 2 replies; 12+ messages in thread
From: Simon Glass @ 2020-01-28 17:07 UTC (permalink / raw)
  To: u-boot

Hi Andre,

On Sat, 25 Jan 2020 at 16:18, André Przywara <andre.przywara@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@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@bootlin.com>
> >> Reviewed-by: Rob Herring <robh@kernel.org>
> >> Signed-off-by: Andre Przywara <andre.przywara@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@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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] dtc: add ability to make nodes conditional on them being referenced
  2020-01-28 17:07     ` Simon Glass
@ 2020-01-28 17:29       ` Andre Przywara
  2020-02-05 17:57       ` sjg at google.com
  1 sibling, 0 replies; 12+ messages in thread
From: Andre Przywara @ 2020-01-28 17:29 UTC (permalink / raw)
  To: u-boot

On Tue, 28 Jan 2020 10:07:18 -0700
Simon Glass <sjg@chromium.org> wrote:

Hi Simon,

> On Sat, 25 Jan 2020 at 16:18, André Przywara <andre.przywara@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@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@bootlin.com>
> > >> Reviewed-by: Rob Herring <robh@kernel.org>
> > >> Signed-off-by: Andre Przywara <andre.przywara@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@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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] dtc: add ability to make nodes conditional on them being referenced
  2020-01-28 17:07     ` Simon Glass
  2020-01-28 17:29       ` Andre Przywara
@ 2020-02-05 17:57       ` sjg at google.com
  1 sibling, 0 replies; 12+ messages in thread
From: sjg at google.com @ 2020-02-05 17:57 UTC (permalink / raw)
  To: u-boot

On Tue, 28 Jan 2020 10:07:18 -0700
Simon Glass <sjg@chromium.org> wrote:

Hi Simon,

> On Sat, 25 Jan 2020 at 16:18, André Przywara <andre.przywara@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@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@bootlin.com>
> > >> Reviewed-by: Rob Herring <robh@kernel.org>
> > >> Signed-off-by: Andre Przywara <andre.przywara@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@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.

Applied to u-boot-dm, thanks!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dtc: add ability to make nodes conditional on them being referenced
  2014-03-16 21:22 ` Heiko Stübner
@ 2014-04-16  7:30   ` David Gibson
  -1 siblings, 0 replies; 12+ messages in thread
From: David Gibson @ 2014-04-16  7:30 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: devicetree, Jon Loeliger, robh+dt, linux-arm-kernel,
	Philipp Zabel, grant.likely, Maxime Ripard, devicetree-compiler


[-- Attachment #1.1: Type: text/plain, Size: 2818 bytes --]

On Sun, Mar 16, 2014 at 10:22:02PM +0100, Heiko Stübner wrote:
> From: Heiko Stuebner <heiko.stuebner@bq.com>
> 
> On i.MX, which carries a lot of pin-groups of which most are unused on
> individual boards, they noticed that this plethora of nodes also results
> in the runtime-lookup-performance also degrading [0].
> 
> A i.MX-specific solution defining the pingroups in the board files but
> using macros to reference the pingroup-data was not well received.
> 
> This patch is trying to solve this issue in a more general way, by
> adding the ability to mark nodes as needing to be referenced somewhere
> in the tree.
> 
> To mark a node a needing to be referenced it must be prefixed with
> /delete-unreferenced/. This makes dtc check the nodes reference-status
> when creating the flattened tree, dropping it if unreferenced.
> 
> For example, the i.MX6SL pingroup
> 
> 	/delete-unreferenced/ pinctrl_ecspi1_1: ecspi1grp-1 {
> 		fsl,pins = <
> 			MX6SL_PAD_ECSPI1_MISO__ECSPI1_MISO 0x100b1
> 			MX6SL_PAD_ECSPI1_MOSI__ECSPI1_MOSI 0x100b1
> 			MX6SL_PAD_ECSPI1_SCLK__ECSPI1_SCLK 0x100b1
> 		>;
> 	};
> 
> would only be included in the dtb if it got referenced somewhere
> as pingroup via
> 
> 	node {
> 		pinctrl-0 <&pinctrl_ecscpi1_1>;
> 	};

Sorry for the delay - I'm pretty busy with non-dtc things.

Concept looks fine to me.  It's perhaps a bit special use case, but
the syntax is well isolated (that is, unlikely to mix badly with other
possible syntax changes), so I'm happy enough to include it for those
who want it.

Maybe change the keyword to /delete-if-unreferenced/, it's a little
bit more accurate.  It's a bit long-winded, but I can't think of a
more succinct term, so it will do.

It needs testcases.

There are a few minor problems in the implementation.

With those things fixed, I'd be happy to apply.

Some detailed comments on the implementation below:

[snip]
> @@ -472,6 +472,8 @@ static void fixup_phandle_references(struct check *c, struct node *dt,
>  
>  		phandle = get_node_phandle(dt, refnode);
>  		*((cell_t *)(prop->val.val + m->offset)) = cpu_to_fdt32(phandle);
> +
> +		reference_node(refnode);

This may have some slightly unexpected behaviour when combined with
/delete-node/ and or /delete-prop/.  At worst it should include the
unnecessary node though, so I don't think it's a big problem.

[snip]
> +struct node *check_node_referenced(struct node *node)

This function needs a different name, since it doesn't actually check
anything (it just triggers a later check).  Maybe
mark_node_needs_reference().

-- 
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

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] dtc: add ability to make nodes conditional on them being referenced
@ 2014-04-16  7:30   ` David Gibson
  0 siblings, 0 replies; 12+ messages in thread
From: David Gibson @ 2014-04-16  7:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Mar 16, 2014 at 10:22:02PM +0100, Heiko St?bner wrote:
> From: Heiko Stuebner <heiko.stuebner@bq.com>
> 
> On i.MX, which carries a lot of pin-groups of which most are unused on
> individual boards, they noticed that this plethora of nodes also results
> in the runtime-lookup-performance also degrading [0].
> 
> A i.MX-specific solution defining the pingroups in the board files but
> using macros to reference the pingroup-data was not well received.
> 
> This patch is trying to solve this issue in a more general way, by
> adding the ability to mark nodes as needing to be referenced somewhere
> in the tree.
> 
> To mark a node a needing to be referenced it must be prefixed with
> /delete-unreferenced/. This makes dtc check the nodes reference-status
> when creating the flattened tree, dropping it if unreferenced.
> 
> For example, the i.MX6SL pingroup
> 
> 	/delete-unreferenced/ pinctrl_ecspi1_1: ecspi1grp-1 {
> 		fsl,pins = <
> 			MX6SL_PAD_ECSPI1_MISO__ECSPI1_MISO 0x100b1
> 			MX6SL_PAD_ECSPI1_MOSI__ECSPI1_MOSI 0x100b1
> 			MX6SL_PAD_ECSPI1_SCLK__ECSPI1_SCLK 0x100b1
> 		>;
> 	};
> 
> would only be included in the dtb if it got referenced somewhere
> as pingroup via
> 
> 	node {
> 		pinctrl-0 <&pinctrl_ecscpi1_1>;
> 	};

Sorry for the delay - I'm pretty busy with non-dtc things.

Concept looks fine to me.  It's perhaps a bit special use case, but
the syntax is well isolated (that is, unlikely to mix badly with other
possible syntax changes), so I'm happy enough to include it for those
who want it.

Maybe change the keyword to /delete-if-unreferenced/, it's a little
bit more accurate.  It's a bit long-winded, but I can't think of a
more succinct term, so it will do.

It needs testcases.

There are a few minor problems in the implementation.

With those things fixed, I'd be happy to apply.

Some detailed comments on the implementation below:

[snip]
> @@ -472,6 +472,8 @@ static void fixup_phandle_references(struct check *c, struct node *dt,
>  
>  		phandle = get_node_phandle(dt, refnode);
>  		*((cell_t *)(prop->val.val + m->offset)) = cpu_to_fdt32(phandle);
> +
> +		reference_node(refnode);

This may have some slightly unexpected behaviour when combined with
/delete-node/ and or /delete-prop/.  At worst it should include the
unnecessary node though, so I don't think it's a big problem.

[snip]
> +struct node *check_node_referenced(struct node *node)

This function needs a different name, since it doesn't actually check
anything (it just triggers a later check).  Maybe
mark_node_needs_reference().

-- 
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140416/3608e549/attachment.sig>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dtc: add ability to make nodes conditional on them being referenced
  2014-03-16 21:22 ` Heiko Stübner
@ 2014-04-15 22:16   ` Heiko Stübner
  -1 siblings, 0 replies; 12+ messages in thread
From: Heiko Stübner @ 2014-04-15 22:16 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Maxime Ripard, Philipp Zabel,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Am Sonntag, 16. März 2014, 22:22:02 schrieb Heiko Stübner:
> From: Heiko Stuebner <heiko.stuebner-K3U4GQvHnyU@public.gmane.org>
> 
> On i.MX, which carries a lot of pin-groups of which most are unused on
> individual boards, they noticed that this plethora of nodes also results
> in the runtime-lookup-performance also degrading [0].
> 
> A i.MX-specific solution defining the pingroups in the board files but
> using macros to reference the pingroup-data was not well received.
> 
> This patch is trying to solve this issue in a more general way, by
> adding the ability to mark nodes as needing to be referenced somewhere
> in the tree.
> 
> To mark a node a needing to be referenced it must be prefixed with
> /delete-unreferenced/. This makes dtc check the nodes reference-status
> when creating the flattened tree, dropping it if unreferenced.
> 
> For example, the i.MX6SL pingroup
> 
> 	/delete-unreferenced/ pinctrl_ecspi1_1: ecspi1grp-1 {
> 		fsl,pins = <
> 			MX6SL_PAD_ECSPI1_MISO__ECSPI1_MISO 0x100b1
> 			MX6SL_PAD_ECSPI1_MOSI__ECSPI1_MOSI 0x100b1
> 			MX6SL_PAD_ECSPI1_SCLK__ECSPI1_SCLK 0x100b1
> 
> 		>;
> 
> 	};
> 
> would only be included in the dtb if it got referenced somewhere
> as pingroup via
> 
> 	node {
> 		pinctrl-0 <&pinctrl_ecscpi1_1>;
> 	};
> 
> [0] http://thread.gmane.org/gmane.linux.ports.arm.kernel/275912/
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner-K3U4GQvHnyU@public.gmane.org>

ping?

Is this ok, bad, something else entirely? :-)


Thanks
Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] dtc: add ability to make nodes conditional on them being referenced
@ 2014-04-15 22:16   ` Heiko Stübner
  0 siblings, 0 replies; 12+ messages in thread
From: Heiko Stübner @ 2014-04-15 22:16 UTC (permalink / raw)
  To: linux-arm-kernel

Am Sonntag, 16. M?rz 2014, 22:22:02 schrieb Heiko St?bner:
> From: Heiko Stuebner <heiko.stuebner@bq.com>
> 
> On i.MX, which carries a lot of pin-groups of which most are unused on
> individual boards, they noticed that this plethora of nodes also results
> in the runtime-lookup-performance also degrading [0].
> 
> A i.MX-specific solution defining the pingroups in the board files but
> using macros to reference the pingroup-data was not well received.
> 
> This patch is trying to solve this issue in a more general way, by
> adding the ability to mark nodes as needing to be referenced somewhere
> in the tree.
> 
> To mark a node a needing to be referenced it must be prefixed with
> /delete-unreferenced/. This makes dtc check the nodes reference-status
> when creating the flattened tree, dropping it if unreferenced.
> 
> For example, the i.MX6SL pingroup
> 
> 	/delete-unreferenced/ pinctrl_ecspi1_1: ecspi1grp-1 {
> 		fsl,pins = <
> 			MX6SL_PAD_ECSPI1_MISO__ECSPI1_MISO 0x100b1
> 			MX6SL_PAD_ECSPI1_MOSI__ECSPI1_MOSI 0x100b1
> 			MX6SL_PAD_ECSPI1_SCLK__ECSPI1_SCLK 0x100b1
> 
> 		>;
> 
> 	};
> 
> would only be included in the dtb if it got referenced somewhere
> as pingroup via
> 
> 	node {
> 		pinctrl-0 <&pinctrl_ecscpi1_1>;
> 	};
> 
> [0] http://thread.gmane.org/gmane.linux.ports.arm.kernel/275912/
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@bq.com>

ping?

Is this ok, bad, something else entirely? :-)


Thanks
Heiko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] dtc: add ability to make nodes conditional on them being referenced
@ 2014-03-16 21:22 ` Heiko Stübner
  0 siblings, 0 replies; 12+ messages in thread
From: Heiko Stübner @ 2014-03-16 21:22 UTC (permalink / raw)
  To: David Gibson, Jon Loeliger
  Cc: Maxime Ripard, Philipp Zabel,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

From: Heiko Stuebner <heiko.stuebner-K3U4GQvHnyU@public.gmane.org>

On i.MX, which carries a lot of pin-groups of which most are unused on
individual boards, they noticed that this plethora of nodes also results
in the runtime-lookup-performance also degrading [0].

A i.MX-specific solution defining the pingroups in the board files but
using macros to reference the pingroup-data was not well received.

This patch is trying to solve this issue in a more general way, by
adding the ability to mark nodes as needing to be referenced somewhere
in the tree.

To mark a node a needing to be referenced it must be prefixed with
/delete-unreferenced/. This makes dtc check the nodes reference-status
when creating the flattened tree, dropping it if unreferenced.

For example, the i.MX6SL pingroup

	/delete-unreferenced/ pinctrl_ecspi1_1: ecspi1grp-1 {
		fsl,pins = <
			MX6SL_PAD_ECSPI1_MISO__ECSPI1_MISO 0x100b1
			MX6SL_PAD_ECSPI1_MOSI__ECSPI1_MOSI 0x100b1
			MX6SL_PAD_ECSPI1_SCLK__ECSPI1_SCLK 0x100b1
		>;
	};

would only be included in the dtb if it got referenced somewhere
as pingroup via

	node {
		pinctrl-0 <&pinctrl_ecscpi1_1>;
	};

[0] http://thread.gmane.org/gmane.linux.ports.arm.kernel/275912/

Signed-off-by: Heiko Stuebner <heiko.stuebner-K3U4GQvHnyU@public.gmane.org>
---
A sample implementation using the in-tree dtc in the linux kernel can
be found at [1] and the very first rfc at [2].

[1] http://comments.gmane.org/gmane.linux.drivers.devicetree/62357
[2] http://comments.gmane.org/gmane.linux.drivers.devicetree/60145

 checks.c     |  2 ++
 dtc-lexer.l  |  7 +++++++
 dtc-parser.y |  5 +++++
 dtc.h        |  4 ++++
 flattree.c   |  3 +++
 livetree.c   | 14 ++++++++++++++
 6 files changed, 35 insertions(+)

diff --git a/checks.c b/checks.c
index 47eda65..a82ba8d 100644
--- a/checks.c
+++ b/checks.c
@@ -472,6 +472,8 @@ static void fixup_phandle_references(struct check *c, struct node *dt,
 
 		phandle = get_node_phandle(dt, refnode);
 		*((cell_t *)(prop->val.val + m->offset)) = cpu_to_fdt32(phandle);
+
+		reference_node(refnode);
 	}
 }
 ERROR(phandle_references, NULL, NULL, fixup_phandle_references, NULL,
diff --git a/dtc-lexer.l b/dtc-lexer.l
index 0821bde..6d7865d 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -140,6 +140,13 @@ static void lexical_error(const char *fmt, ...);
 			return DT_DEL_NODE;
 		}
 
+<*>"/delete-unreferenced/"	{
+			DPRINT("Keyword: /delete-unreferenced/\n");
+			DPRINT("<PROPNODENAME>\n");
+			BEGIN(PROPNODENAME);
+			return DT_DEL_UNREFERENCED;
+		}
+
 <*>{LABEL}:	{
 			DPRINT("Label: %s\n", yytext);
 			yylval.labelref = xstrdup(yytext);
diff --git a/dtc-parser.y b/dtc-parser.y
index ea57e0a..a203bed 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -60,6 +60,7 @@ extern bool treesource_error;
 %token DT_BITS
 %token DT_DEL_PROP
 %token DT_DEL_NODE
+%token DT_DEL_UNREFERENCED
 %token <propnodename> DT_PROPNODENAME
 %token <integer> DT_LITERAL
 %token <integer> DT_CHAR_LITERAL
@@ -450,6 +451,10 @@ subnode:
 		{
 			$$ = name_node(build_node_delete(), $2);
 		}
+	| DT_DEL_UNREFERENCED subnode
+		{
+			$$ = check_node_referenced($2);
+		}
 	| DT_LABEL subnode
 		{
 			add_label(&$2->labels, $1);
diff --git a/dtc.h b/dtc.h
index 20de073..ff45059 100644
--- a/dtc.h
+++ b/dtc.h
@@ -158,6 +158,8 @@ struct node {
 	int addr_cells, size_cells;
 
 	struct label *labels;
+
+	int needs_reference, is_referenced;
 };
 
 #define for_each_label_withdel(l0, l) \
@@ -192,6 +194,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 *check_node_referenced(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);
 
diff --git a/flattree.c b/flattree.c
index bd99fa2..105e8fa 100644
--- a/flattree.c
+++ b/flattree.c
@@ -266,6 +266,9 @@ static void flatten_tree(struct node *tree, struct emitter *emit,
 	if (tree->deleted)
 		return;
 
+	if (tree->needs_reference && !tree->is_referenced)
+		return;
+
 	emit->beginnode(etarget, tree->labels);
 
 	if (vi->flags & FTF_FULLPATH)
diff --git a/livetree.c b/livetree.c
index b61465f..98bb33d 100644
--- a/livetree.c
+++ b/livetree.c
@@ -134,6 +134,20 @@ struct node *name_node(struct node *node, char *name)
 	return node;
 }
 
+struct node *check_node_referenced(struct node *node)
+{
+	node->needs_reference = 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;
-- 
1.9.0


--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] dtc: add ability to make nodes conditional on them being referenced
@ 2014-03-16 21:22 ` Heiko Stübner
  0 siblings, 0 replies; 12+ messages in thread
From: Heiko Stübner @ 2014-03-16 21:22 UTC (permalink / raw)
  To: linux-arm-kernel

From: Heiko Stuebner <heiko.stuebner@bq.com>

On i.MX, which carries a lot of pin-groups of which most are unused on
individual boards, they noticed that this plethora of nodes also results
in the runtime-lookup-performance also degrading [0].

A i.MX-specific solution defining the pingroups in the board files but
using macros to reference the pingroup-data was not well received.

This patch is trying to solve this issue in a more general way, by
adding the ability to mark nodes as needing to be referenced somewhere
in the tree.

To mark a node a needing to be referenced it must be prefixed with
/delete-unreferenced/. This makes dtc check the nodes reference-status
when creating the flattened tree, dropping it if unreferenced.

For example, the i.MX6SL pingroup

	/delete-unreferenced/ pinctrl_ecspi1_1: ecspi1grp-1 {
		fsl,pins = <
			MX6SL_PAD_ECSPI1_MISO__ECSPI1_MISO 0x100b1
			MX6SL_PAD_ECSPI1_MOSI__ECSPI1_MOSI 0x100b1
			MX6SL_PAD_ECSPI1_SCLK__ECSPI1_SCLK 0x100b1
		>;
	};

would only be included in the dtb if it got referenced somewhere
as pingroup via

	node {
		pinctrl-0 <&pinctrl_ecscpi1_1>;
	};

[0] http://thread.gmane.org/gmane.linux.ports.arm.kernel/275912/

Signed-off-by: Heiko Stuebner <heiko.stuebner@bq.com>
---
A sample implementation using the in-tree dtc in the linux kernel can
be found at [1] and the very first rfc at [2].

[1] http://comments.gmane.org/gmane.linux.drivers.devicetree/62357
[2] http://comments.gmane.org/gmane.linux.drivers.devicetree/60145

 checks.c     |  2 ++
 dtc-lexer.l  |  7 +++++++
 dtc-parser.y |  5 +++++
 dtc.h        |  4 ++++
 flattree.c   |  3 +++
 livetree.c   | 14 ++++++++++++++
 6 files changed, 35 insertions(+)

diff --git a/checks.c b/checks.c
index 47eda65..a82ba8d 100644
--- a/checks.c
+++ b/checks.c
@@ -472,6 +472,8 @@ static void fixup_phandle_references(struct check *c, struct node *dt,
 
 		phandle = get_node_phandle(dt, refnode);
 		*((cell_t *)(prop->val.val + m->offset)) = cpu_to_fdt32(phandle);
+
+		reference_node(refnode);
 	}
 }
 ERROR(phandle_references, NULL, NULL, fixup_phandle_references, NULL,
diff --git a/dtc-lexer.l b/dtc-lexer.l
index 0821bde..6d7865d 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -140,6 +140,13 @@ static void lexical_error(const char *fmt, ...);
 			return DT_DEL_NODE;
 		}
 
+<*>"/delete-unreferenced/"	{
+			DPRINT("Keyword: /delete-unreferenced/\n");
+			DPRINT("<PROPNODENAME>\n");
+			BEGIN(PROPNODENAME);
+			return DT_DEL_UNREFERENCED;
+		}
+
 <*>{LABEL}:	{
 			DPRINT("Label: %s\n", yytext);
 			yylval.labelref = xstrdup(yytext);
diff --git a/dtc-parser.y b/dtc-parser.y
index ea57e0a..a203bed 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -60,6 +60,7 @@ extern bool treesource_error;
 %token DT_BITS
 %token DT_DEL_PROP
 %token DT_DEL_NODE
+%token DT_DEL_UNREFERENCED
 %token <propnodename> DT_PROPNODENAME
 %token <integer> DT_LITERAL
 %token <integer> DT_CHAR_LITERAL
@@ -450,6 +451,10 @@ subnode:
 		{
 			$$ = name_node(build_node_delete(), $2);
 		}
+	| DT_DEL_UNREFERENCED subnode
+		{
+			$$ = check_node_referenced($2);
+		}
 	| DT_LABEL subnode
 		{
 			add_label(&$2->labels, $1);
diff --git a/dtc.h b/dtc.h
index 20de073..ff45059 100644
--- a/dtc.h
+++ b/dtc.h
@@ -158,6 +158,8 @@ struct node {
 	int addr_cells, size_cells;
 
 	struct label *labels;
+
+	int needs_reference, is_referenced;
 };
 
 #define for_each_label_withdel(l0, l) \
@@ -192,6 +194,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 *check_node_referenced(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);
 
diff --git a/flattree.c b/flattree.c
index bd99fa2..105e8fa 100644
--- a/flattree.c
+++ b/flattree.c
@@ -266,6 +266,9 @@ static void flatten_tree(struct node *tree, struct emitter *emit,
 	if (tree->deleted)
 		return;
 
+	if (tree->needs_reference && !tree->is_referenced)
+		return;
+
 	emit->beginnode(etarget, tree->labels);
 
 	if (vi->flags & FTF_FULLPATH)
diff --git a/livetree.c b/livetree.c
index b61465f..98bb33d 100644
--- a/livetree.c
+++ b/livetree.c
@@ -134,6 +134,20 @@ struct node *name_node(struct node *node, char *name)
 	return node;
 }
 
+struct node *check_node_referenced(struct node *node)
+{
+	node->needs_reference = 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;
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-02-05 17:57 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-21 10:23 [PATCH] dtc: add ability to make nodes conditional on them being referenced Andre Przywara
2020-01-25 16:20 ` Tom Rini
2020-01-25 22:18   ` André Przywara
2020-01-28 17:07     ` Simon Glass
2020-01-28 17:29       ` Andre Przywara
2020-02-05 17:57       ` sjg at google.com
  -- strict thread matches above, loose matches on Subject: below --
2014-03-16 21:22 Heiko Stübner
2014-03-16 21:22 ` Heiko Stübner
2014-04-15 22:16 ` Heiko Stübner
2014-04-15 22:16   ` Heiko Stübner
2014-04-16  7:30 ` David Gibson
2014-04-16  7:30   ` David Gibson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.