mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/2] of: Add for_each_compatible_node_from iterator
@ 2015-12-19  0:13 Trent Piepho
  2015-12-19  0:18 ` [PATCH 2/2] of: Add of_property_for_each_phandle() iterator Trent Piepho
  2016-01-04  8:32 ` [PATCH 1/2] of: Add for_each_compatible_node_from iterator Sascha Hauer
  0 siblings, 2 replies; 14+ messages in thread
From: Trent Piepho @ 2015-12-19  0:13 UTC (permalink / raw)
  To: barebox

The existing iterator for_each_compatible_node() searches for each
compatible node starting from the root of the loaded device tree.
This means it only works on the barebox device tree and not the tree
to be passed to the Linux kernel, which is what an of_fixup would
probably want to use.

This adds for_each_compatible_node_from(), which takes an additional
parameter of a root to search from.  This could be the device tree to
be used for the kernel.  It could also be used to search just a
subtree.

Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
---

It's possible the fixups in cm_cogent_fixup() and hb_fixup() should
be using this.  It's not clear to me if they want to modify the barebox
device tree or the Linux device tree or both.

 include/of.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/of.h b/include/of.h
index e60fe89..e65a8e8 100644
--- a/include/of.h
+++ b/include/of.h
@@ -641,9 +641,13 @@ static inline struct device_node *of_find_node_by_path_or_alias(
 #define for_each_node_by_name_from(dn, root, name) \
 	for (dn = of_find_node_by_name(root, name); dn; \
 	     dn = of_find_node_by_name(dn, name))
-#define for_each_compatible_node(dn, type, compatible) \
-	for (dn = of_find_compatible_node(NULL, type, compatible); dn; \
+/* Iterate over compatible nodes starting from given root */
+#define for_each_compatible_node_from(dn, root, type, compatible) \
+	for (dn = of_find_compatible_node(root, type, compatible); dn; \
 	     dn = of_find_compatible_node(dn, type, compatible))
+/* Iterate over compatible nodes in default device tree */
+#define for_each_compatible_node(dn, type, compatible) \
+        for_each_compatible_node_from(dn, NULL, type, compatible
 static inline struct device_node *of_find_matching_node(
 	struct device_node *from,
 	const struct of_device_id *matches)
-- 
1.8.3.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 2/2] of: Add of_property_for_each_phandle() iterator
  2015-12-19  0:13 [PATCH 1/2] of: Add for_each_compatible_node_from iterator Trent Piepho
@ 2015-12-19  0:18 ` Trent Piepho
  2016-01-04  8:32 ` [PATCH 1/2] of: Add for_each_compatible_node_from iterator Sascha Hauer
  1 sibling, 0 replies; 14+ messages in thread
From: Trent Piepho @ 2015-12-19  0:18 UTC (permalink / raw)
  To: barebox

This is like of_property_for_each_{string,u32} but loops over a list
of phandles instead of strings or ints.

Returns the node the handle points to as that's generally more useful
than the handle value itself.

Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
---
 include/of.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/of.h b/include/of.h
index e65a8e8..18c92d7 100644
--- a/include/of.h
+++ b/include/of.h
@@ -736,6 +736,17 @@ static inline int of_property_read_u32(const struct device_node *np,
 		s;						\
 		s = of_prop_next_string(prop, s))
 
+/*
+ * struct device_node *n;
+ *
+ * of_property_for_each_phandle(np, root, "propname", n)
+ *         printk("phandle points to: %s\n", n->full_name);
+ */
+#define of_property_for_each_phandle(np, root, propname, n)	\
+	for (int _i = 0; 					\
+	     (n = of_parse_phandle_from(np, root, propname, _i));\
+	     _i++)
+
 static inline int of_property_write_u8(struct device_node *np,
 				       const char *propname, u8 value)
 {
-- 
1.8.3.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 1/2] of: Add for_each_compatible_node_from iterator
  2015-12-19  0:13 [PATCH 1/2] of: Add for_each_compatible_node_from iterator Trent Piepho
  2015-12-19  0:18 ` [PATCH 2/2] of: Add of_property_for_each_phandle() iterator Trent Piepho
@ 2016-01-04  8:32 ` Sascha Hauer
  2016-01-04 19:01   ` [PATCH 1/2] OF: fix typo in for_each_compatible_node() Trent Piepho
                     ` (2 more replies)
  1 sibling, 3 replies; 14+ messages in thread
From: Sascha Hauer @ 2016-01-04  8:32 UTC (permalink / raw)
  To: Trent Piepho; +Cc: barebox

On Sat, Dec 19, 2015 at 12:13:59AM +0000, Trent Piepho wrote:
> The existing iterator for_each_compatible_node() searches for each
> compatible node starting from the root of the loaded device tree.
> This means it only works on the barebox device tree and not the tree
> to be passed to the Linux kernel, which is what an of_fixup would
> probably want to use.
> 
> This adds for_each_compatible_node_from(), which takes an additional
> parameter of a root to search from.  This could be the device tree to
> be used for the kernel.  It could also be used to search just a
> subtree.
> 
> Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>

Applied, thanks

> ---
> 
> It's possible the fixups in cm_cogent_fixup() and hb_fixup() should
> be using this.  It's not clear to me if they want to modify the barebox
> device tree or the Linux device tree or both.

It's always the Linux device tree that is fixed up in the OF fixups.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 1/2] OF: fix typo in for_each_compatible_node()
  2016-01-04  8:32 ` [PATCH 1/2] of: Add for_each_compatible_node_from iterator Sascha Hauer
@ 2016-01-04 19:01   ` Trent Piepho
  2016-01-05  7:49     ` Sascha Hauer
  2016-01-04 19:02   ` [PATCH 2/2] OF: Fix fixups to fix Linux DT instead of Barebox DT Trent Piepho
  2016-01-04 19:07   ` [PATCH 1/2] of: Add for_each_compatible_node_from iterator Trent Piepho
  2 siblings, 1 reply; 14+ messages in thread
From: Trent Piepho @ 2016-01-04 19:01 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Missing closing parenthesis.

Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
---
 include/of.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/of.h b/include/of.h
index 18c92d7..75cc3c1 100644
--- a/include/of.h
+++ b/include/of.h
@@ -647,7 +647,7 @@ static inline struct device_node *of_find_node_by_path_or_alias(
 	     dn = of_find_compatible_node(dn, type, compatible))
 /* Iterate over compatible nodes in default device tree */
 #define for_each_compatible_node(dn, type, compatible) \
-        for_each_compatible_node_from(dn, NULL, type, compatible
+        for_each_compatible_node_from(dn, NULL, type, compatible)
 static inline struct device_node *of_find_matching_node(
 	struct device_node *from,
 	const struct of_device_id *matches)
-- 
1.8.3.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 2/2] OF: Fix fixups to fix Linux DT instead of Barebox DT
  2016-01-04  8:32 ` [PATCH 1/2] of: Add for_each_compatible_node_from iterator Sascha Hauer
  2016-01-04 19:01   ` [PATCH 1/2] OF: fix typo in for_each_compatible_node() Trent Piepho
@ 2016-01-04 19:02   ` Trent Piepho
  2016-01-05  7:50     ` Sascha Hauer
  2016-01-04 19:07   ` [PATCH 1/2] of: Add for_each_compatible_node_from iterator Trent Piepho
  2 siblings, 1 reply; 14+ messages in thread
From: Trent Piepho @ 2016-01-04 19:02 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

OF fixups cm_cogent_fixup() and hb_fixup() are supposed to modify the
Linux device tree.  And they did originally, but commit
e520a8cc463760d21890b35218e4dac817e7c7e7 changed them to use
for_each_compatible_node(), which iterates through the Barebox DT.
Use new for_each_compatible_node_from() to specify the Linux DT root
as the start point.

Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
---
 arch/arm/boards/at91sam9x5ek/hw_version.c | 2 +-
 arch/arm/boards/highbank/init.c           | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boards/at91sam9x5ek/hw_version.c b/arch/arm/boards/at91sam9x5ek/hw_version.c
index 37eb1f8..2f84d82 100644
--- a/arch/arm/boards/at91sam9x5ek/hw_version.c
+++ b/arch/arm/boards/at91sam9x5ek/hw_version.c
@@ -235,7 +235,7 @@ static int cm_cogent_fixup(struct device_node *root, void *unused)
 	int ret;
 	struct device_node *node;
 
-	for_each_compatible_node(node, NULL, "atmel,hsmci") {
+	for_each_compatible_node_from(node, root, NULL, "atmel,hsmci") {
 		struct device_node *slotnode =
 			of_get_child_by_name(node, "slot");
 		if (!slotnode)
diff --git a/arch/arm/boards/highbank/init.c b/arch/arm/boards/highbank/init.c
index a0d4b30..1cb02e6 100644
--- a/arch/arm/boards/highbank/init.c
+++ b/arch/arm/boards/highbank/init.c
@@ -35,13 +35,13 @@ static int hb_fixup(struct device_node *root, void *unused)
 	__be32 latency;
 
 	if (!(reg & HB_PWRDOM_STAT_SATA)) {
-		for_each_compatible_node(node, NULL, "calxeda,hb-ahci")
+		for_each_compatible_node_from(node, root, NULL, "calxeda,hb-ahci")
 			of_set_property(node, "status", "disabled",
 					sizeof("disabled"), 1);
 	}
 
 	if (!(reg & HB_PWRDOM_STAT_EMMC)) {
-		for_each_compatible_node(node, NULL, "calxeda,hb-sdhci")
+		for_each_compatible_node_from(node, root, NULL, "calxeda,hb-sdhci")
 			of_set_property(node, "status", "disabled",
 					sizeof("disabled"), 1);
 	}
-- 
1.8.3.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 1/2] of: Add for_each_compatible_node_from iterator
  2016-01-04  8:32 ` [PATCH 1/2] of: Add for_each_compatible_node_from iterator Sascha Hauer
  2016-01-04 19:01   ` [PATCH 1/2] OF: fix typo in for_each_compatible_node() Trent Piepho
  2016-01-04 19:02   ` [PATCH 2/2] OF: Fix fixups to fix Linux DT instead of Barebox DT Trent Piepho
@ 2016-01-04 19:07   ` Trent Piepho
  2016-01-05  7:58     ` Sascha Hauer
  2 siblings, 1 reply; 14+ messages in thread
From: Trent Piepho @ 2016-01-04 19:07 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Mon, 2016-01-04 at 09:32 +0100, Sascha Hauer wrote:
> On Sat, Dec 19, 2015 at 12:13:59AM +0000, Trent Piepho wrote:
> > The existing iterator for_each_compatible_node() searches for each
> > compatible node starting from the root of the loaded device tree.
> > This means it only works on the barebox device tree and not the tree
> > to be passed to the Linux kernel, which is what an of_fixup would
> > probably want to use.
> > 
> > This adds for_each_compatible_node_from(), which takes an additional
> > parameter of a root to search from.  This could be the device tree to
> > be used for the kernel.  It could also be used to search just a
> > subtree.
> > 
> > Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
> 
> Applied, thanks
> 
> > ---
> > 
> > It's possible the fixups in cm_cogent_fixup() and hb_fixup() should
> > be using this.  It's not clear to me if they want to modify the barebox
> > device tree or the Linux device tree or both.
> 
> It's always the Linux device tree that is fixed up in the OF fixups.

Sent patch to fix them.

Couldn't one also use the of fixup system to modify the barebox DT?  In
order to support multiple board variants, I added DT nodes that specify
what nodes should be enabled and/or disabled for different board
versions.  An OF fixup applies this to the Linux DT.  I haven't had to
modify the barebox DT for different boards but anticipate that happening
for the next board and I was planning to use the same system.
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 1/2] OF: fix typo in for_each_compatible_node()
  2016-01-04 19:01   ` [PATCH 1/2] OF: fix typo in for_each_compatible_node() Trent Piepho
@ 2016-01-05  7:49     ` Sascha Hauer
  0 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2016-01-05  7:49 UTC (permalink / raw)
  To: Trent Piepho; +Cc: barebox

On Mon, Jan 04, 2016 at 07:01:21PM +0000, Trent Piepho wrote:
> Missing closing parenthesis.
> 
> Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>

Applied, thanks.

BTW as long as the patches you fixup are in the next branch you can
commit the fixes with --fixup. This allows me to squash the fixup into
the correct patch without much thinking.

Sascha

> ---
>  include/of.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/of.h b/include/of.h
> index 18c92d7..75cc3c1 100644
> --- a/include/of.h
> +++ b/include/of.h
> @@ -647,7 +647,7 @@ static inline struct device_node *of_find_node_by_path_or_alias(
>  	     dn = of_find_compatible_node(dn, type, compatible))
>  /* Iterate over compatible nodes in default device tree */
>  #define for_each_compatible_node(dn, type, compatible) \
> -        for_each_compatible_node_from(dn, NULL, type, compatible
> +        for_each_compatible_node_from(dn, NULL, type, compatible)
>  static inline struct device_node *of_find_matching_node(
>  	struct device_node *from,
>  	const struct of_device_id *matches)
> -- 
> 1.8.3.1
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 2/2] OF: Fix fixups to fix Linux DT instead of Barebox DT
  2016-01-04 19:02   ` [PATCH 2/2] OF: Fix fixups to fix Linux DT instead of Barebox DT Trent Piepho
@ 2016-01-05  7:50     ` Sascha Hauer
  0 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2016-01-05  7:50 UTC (permalink / raw)
  To: Trent Piepho; +Cc: barebox

On Mon, Jan 04, 2016 at 07:02:34PM +0000, Trent Piepho wrote:
> OF fixups cm_cogent_fixup() and hb_fixup() are supposed to modify the
> Linux device tree.  And they did originally, but commit
> e520a8cc463760d21890b35218e4dac817e7c7e7 changed them to use
> for_each_compatible_node(), which iterates through the Barebox DT.
> Use new for_each_compatible_node_from() to specify the Linux DT root
> as the start point.
> 
> Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
> ---
>  arch/arm/boards/at91sam9x5ek/hw_version.c | 2 +-
>  arch/arm/boards/highbank/init.c           | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 1/2] of: Add for_each_compatible_node_from iterator
  2016-01-04 19:07   ` [PATCH 1/2] of: Add for_each_compatible_node_from iterator Trent Piepho
@ 2016-01-05  7:58     ` Sascha Hauer
  2016-01-05  8:05       ` Yegor Yefremov
  2016-01-05 18:58       ` Trent Piepho
  0 siblings, 2 replies; 14+ messages in thread
From: Sascha Hauer @ 2016-01-05  7:58 UTC (permalink / raw)
  To: Trent Piepho; +Cc: barebox

On Mon, Jan 04, 2016 at 07:07:27PM +0000, Trent Piepho wrote:
> On Mon, 2016-01-04 at 09:32 +0100, Sascha Hauer wrote:
> > On Sat, Dec 19, 2015 at 12:13:59AM +0000, Trent Piepho wrote:
> > > The existing iterator for_each_compatible_node() searches for each
> > > compatible node starting from the root of the loaded device tree.
> > > This means it only works on the barebox device tree and not the tree
> > > to be passed to the Linux kernel, which is what an of_fixup would
> > > probably want to use.
> > > 
> > > This adds for_each_compatible_node_from(), which takes an additional
> > > parameter of a root to search from.  This could be the device tree to
> > > be used for the kernel.  It could also be used to search just a
> > > subtree.
> > > 
> > > Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
> > 
> > Applied, thanks
> > 
> > > ---
> > > 
> > > It's possible the fixups in cm_cogent_fixup() and hb_fixup() should
> > > be using this.  It's not clear to me if they want to modify the barebox
> > > device tree or the Linux device tree or both.
> > 
> > It's always the Linux device tree that is fixed up in the OF fixups.
> 
> Sent patch to fix them.
> 
> Couldn't one also use the of fixup system to modify the barebox DT?  In
> order to support multiple board variants, I added DT nodes that specify
> what nodes should be enabled and/or disabled for different board
> versions.  An OF fixup applies this to the Linux DT.  I haven't had to
> modify the barebox DT for different boards but anticipate that happening
> for the next board and I was planning to use the same system.

I think you don't need the fixup system to accomplish that. Just hook up
to an initcall early enough and modify the barebox device tree. It
shouldn't be necessary to register a callback first and then wait for
its execution.
Are you aware of device tree overlays? We planned to merge support for
them into barebox once the official device tree compiler supports them.
Unfortunately this hasn't happened yet.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 1/2] of: Add for_each_compatible_node_from iterator
  2016-01-05  7:58     ` Sascha Hauer
@ 2016-01-05  8:05       ` Yegor Yefremov
  2016-01-05  8:20         ` Sascha Hauer
  2016-01-05 18:58       ` Trent Piepho
  1 sibling, 1 reply; 14+ messages in thread
From: Yegor Yefremov @ 2016-01-05  8:05 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox, Trent Piepho

On Tue, Jan 5, 2016 at 8:58 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Mon, Jan 04, 2016 at 07:07:27PM +0000, Trent Piepho wrote:
>> On Mon, 2016-01-04 at 09:32 +0100, Sascha Hauer wrote:
>> > On Sat, Dec 19, 2015 at 12:13:59AM +0000, Trent Piepho wrote:
>> > > The existing iterator for_each_compatible_node() searches for each
>> > > compatible node starting from the root of the loaded device tree.
>> > > This means it only works on the barebox device tree and not the tree
>> > > to be passed to the Linux kernel, which is what an of_fixup would
>> > > probably want to use.
>> > >
>> > > This adds for_each_compatible_node_from(), which takes an additional
>> > > parameter of a root to search from.  This could be the device tree to
>> > > be used for the kernel.  It could also be used to search just a
>> > > subtree.
>> > >
>> > > Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
>> >
>> > Applied, thanks
>> >
>> > > ---
>> > >
>> > > It's possible the fixups in cm_cogent_fixup() and hb_fixup() should
>> > > be using this.  It's not clear to me if they want to modify the barebox
>> > > device tree or the Linux device tree or both.
>> >
>> > It's always the Linux device tree that is fixed up in the OF fixups.
>>
>> Sent patch to fix them.
>>
>> Couldn't one also use the of fixup system to modify the barebox DT?  In
>> order to support multiple board variants, I added DT nodes that specify
>> what nodes should be enabled and/or disabled for different board
>> versions.  An OF fixup applies this to the Linux DT.  I haven't had to
>> modify the barebox DT for different boards but anticipate that happening
>> for the next board and I was planning to use the same system.
>
> I think you don't need the fixup system to accomplish that. Just hook up
> to an initcall early enough and modify the barebox device tree. It
> shouldn't be necessary to register a callback first and then wait for
> its execution.

What initcall can be used to change the device tree, that is already
loaded into memory and before Linux is started?

My goal is (as soon as FIT support is merged) to load corresponding
blob and change MACs.

Yegor

> Are you aware of device tree overlays? We planned to merge support for
> them into barebox once the official device tree compiler supports them.
> Unfortunately this hasn't happened yet.
>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 1/2] of: Add for_each_compatible_node_from iterator
  2016-01-05  8:05       ` Yegor Yefremov
@ 2016-01-05  8:20         ` Sascha Hauer
  2016-01-05  8:30           ` Yegor Yefremov
  0 siblings, 1 reply; 14+ messages in thread
From: Sascha Hauer @ 2016-01-05  8:20 UTC (permalink / raw)
  To: Yegor Yefremov; +Cc: barebox, Trent Piepho

On Tue, Jan 05, 2016 at 09:05:35AM +0100, Yegor Yefremov wrote:
> On Tue, Jan 5, 2016 at 8:58 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Mon, Jan 04, 2016 at 07:07:27PM +0000, Trent Piepho wrote:
> >> On Mon, 2016-01-04 at 09:32 +0100, Sascha Hauer wrote:
> >> > On Sat, Dec 19, 2015 at 12:13:59AM +0000, Trent Piepho wrote:
> >> > > The existing iterator for_each_compatible_node() searches for each
> >> > > compatible node starting from the root of the loaded device tree.
> >> > > This means it only works on the barebox device tree and not the tree
> >> > > to be passed to the Linux kernel, which is what an of_fixup would
> >> > > probably want to use.
> >> > >
> >> > > This adds for_each_compatible_node_from(), which takes an additional
> >> > > parameter of a root to search from.  This could be the device tree to
> >> > > be used for the kernel.  It could also be used to search just a
> >> > > subtree.
> >> > >
> >> > > Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
> >> >
> >> > Applied, thanks
> >> >
> >> > > ---
> >> > >
> >> > > It's possible the fixups in cm_cogent_fixup() and hb_fixup() should
> >> > > be using this.  It's not clear to me if they want to modify the barebox
> >> > > device tree or the Linux device tree or both.
> >> >
> >> > It's always the Linux device tree that is fixed up in the OF fixups.
> >>
> >> Sent patch to fix them.
> >>
> >> Couldn't one also use the of fixup system to modify the barebox DT?  In
> >> order to support multiple board variants, I added DT nodes that specify
> >> what nodes should be enabled and/or disabled for different board
> >> versions.  An OF fixup applies this to the Linux DT.  I haven't had to
> >> modify the barebox DT for different boards but anticipate that happening
> >> for the next board and I was planning to use the same system.
> >
> > I think you don't need the fixup system to accomplish that. Just hook up
> > to an initcall early enough and modify the barebox device tree. It
> > shouldn't be necessary to register a callback first and then wait for
> > its execution.
> 
> What initcall can be used to change the device tree, that is already
> loaded into memory and before Linux is started?

You want to change the device tree that is passed to Linux, right? In
this case you can use of_register_fixup(). We were talking about
changing the device tree that barebox uses for itself. For this
everything after core_initcall will work. It should be early enough
though so that the device that is ought to be changed has not probed, so
I suggest doing it as early as possible, that would be
postcore_initcall.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 1/2] of: Add for_each_compatible_node_from iterator
  2016-01-05  8:20         ` Sascha Hauer
@ 2016-01-05  8:30           ` Yegor Yefremov
  0 siblings, 0 replies; 14+ messages in thread
From: Yegor Yefremov @ 2016-01-05  8:30 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox, Trent Piepho

On Tue, Jan 5, 2016 at 9:20 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Tue, Jan 05, 2016 at 09:05:35AM +0100, Yegor Yefremov wrote:
>> On Tue, Jan 5, 2016 at 8:58 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> > On Mon, Jan 04, 2016 at 07:07:27PM +0000, Trent Piepho wrote:
>> >> On Mon, 2016-01-04 at 09:32 +0100, Sascha Hauer wrote:
>> >> > On Sat, Dec 19, 2015 at 12:13:59AM +0000, Trent Piepho wrote:
>> >> > > The existing iterator for_each_compatible_node() searches for each
>> >> > > compatible node starting from the root of the loaded device tree.
>> >> > > This means it only works on the barebox device tree and not the tree
>> >> > > to be passed to the Linux kernel, which is what an of_fixup would
>> >> > > probably want to use.
>> >> > >
>> >> > > This adds for_each_compatible_node_from(), which takes an additional
>> >> > > parameter of a root to search from.  This could be the device tree to
>> >> > > be used for the kernel.  It could also be used to search just a
>> >> > > subtree.
>> >> > >
>> >> > > Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
>> >> >
>> >> > Applied, thanks
>> >> >
>> >> > > ---
>> >> > >
>> >> > > It's possible the fixups in cm_cogent_fixup() and hb_fixup() should
>> >> > > be using this.  It's not clear to me if they want to modify the barebox
>> >> > > device tree or the Linux device tree or both.
>> >> >
>> >> > It's always the Linux device tree that is fixed up in the OF fixups.
>> >>
>> >> Sent patch to fix them.
>> >>
>> >> Couldn't one also use the of fixup system to modify the barebox DT?  In
>> >> order to support multiple board variants, I added DT nodes that specify
>> >> what nodes should be enabled and/or disabled for different board
>> >> versions.  An OF fixup applies this to the Linux DT.  I haven't had to
>> >> modify the barebox DT for different boards but anticipate that happening
>> >> for the next board and I was planning to use the same system.
>> >
>> > I think you don't need the fixup system to accomplish that. Just hook up
>> > to an initcall early enough and modify the barebox device tree. It
>> > shouldn't be necessary to register a callback first and then wait for
>> > its execution.
>>
>> What initcall can be used to change the device tree, that is already
>> loaded into memory and before Linux is started?
>
> You want to change the device tree that is passed to Linux, right? In
> this case you can use of_register_fixup(). We were talking about
> changing the device tree that barebox uses for itself. For this
> everything after core_initcall will work. It should be early enough
> though so that the device that is ought to be changed has not probed, so
> I suggest doing it as early as possible, that would be
> postcore_initcall.

OK. Thanks.

Yegor

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 1/2] of: Add for_each_compatible_node_from iterator
  2016-01-05  7:58     ` Sascha Hauer
  2016-01-05  8:05       ` Yegor Yefremov
@ 2016-01-05 18:58       ` Trent Piepho
  2016-01-07  7:34         ` Sascha Hauer
  1 sibling, 1 reply; 14+ messages in thread
From: Trent Piepho @ 2016-01-05 18:58 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Tue, 2016-01-05 at 08:58 +0100, Sascha Hauer wrote:
> On Mon, Jan 04, 2016 at 07:07:27PM +0000, Trent Piepho wrote: 
> > Couldn't one also use the of fixup system to modify the barebox DT?  In
> > order to support multiple board variants, I added DT nodes that specify
> > what nodes should be enabled and/or disabled for different board
> > versions.  An OF fixup applies this to the Linux DT.  I haven't had to
> > modify the barebox DT for different boards but anticipate that happening
> > for the next board and I was planning to use the same system.
> 
> I think you don't need the fixup system to accomplish that. Just hook up
> to an initcall early enough and modify the barebox device tree. It
> shouldn't be necessary to register a callback first and then wait for
> its execution.

My thought was that the fixup already registered for the Linux device
tree would also cover the barebox tree, including things like oftree -l
to load a new one.  Calling the fixup function from an initcall wouldn't
get that.  I guess I'll see what happens when I need that feature.

> Are you aware of device tree overlays? We planned to merge support for
> them into barebox once the official device tree compiler supports them.
> Unfortunately this hasn't happened yet.

I thought of that, and also using include files and generating a bunch
of device trees, but decided against it.  It would be a lot of work to
add support for generating multiple device trees into the kernel build
system and buildroot which is driving everything.  And then you have a
pile of device tree/overlay files to keep track of.  And barebox has to
figure out which one to load for the kernel.

So I made a system where everything is supported by one device tree.

I also added support for specifying a tree to boot barebox with, hence
my patches for arm boot calling convention, but decided having the
xloader find a device tree fragment and pass it to the main barebox was
going to be too much of a pain.  Thus the one tree to rule them all
solution.
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 1/2] of: Add for_each_compatible_node_from iterator
  2016-01-05 18:58       ` Trent Piepho
@ 2016-01-07  7:34         ` Sascha Hauer
  0 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2016-01-07  7:34 UTC (permalink / raw)
  To: Trent Piepho; +Cc: barebox

On Tue, Jan 05, 2016 at 06:58:01PM +0000, Trent Piepho wrote:
> On Tue, 2016-01-05 at 08:58 +0100, Sascha Hauer wrote:
> > On Mon, Jan 04, 2016 at 07:07:27PM +0000, Trent Piepho wrote: 
> > > Couldn't one also use the of fixup system to modify the barebox DT?  In
> > > order to support multiple board variants, I added DT nodes that specify
> > > what nodes should be enabled and/or disabled for different board
> > > versions.  An OF fixup applies this to the Linux DT.  I haven't had to
> > > modify the barebox DT for different boards but anticipate that happening
> > > for the next board and I was planning to use the same system.
> > 
> > I think you don't need the fixup system to accomplish that. Just hook up
> > to an initcall early enough and modify the barebox device tree. It
> > shouldn't be necessary to register a callback first and then wait for
> > its execution.
> 
> My thought was that the fixup already registered for the Linux device
> tree would also cover the barebox tree, including things like oftree -l
> to load a new one.  Calling the fixup function from an initcall wouldn't
> get that.  I guess I'll see what happens when I need that feature.

At the moment the tree passed to of_fix_tree is fixed, so you can always
call of_fix_tree with the internal device tree to execute the fixups.
Using fixups also means that every external device tree loaded to start
the kernel also gets the fixups applied. If that's what you want then
indeed the fixups are for your usecase.

> 
> > Are you aware of device tree overlays? We planned to merge support for
> > them into barebox once the official device tree compiler supports them.
> > Unfortunately this hasn't happened yet.
> 
> I thought of that, and also using include files and generating a bunch
> of device trees, but decided against it.  It would be a lot of work to
> add support for generating multiple device trees into the kernel build
> system and buildroot which is driving everything.  And then you have a
> pile of device tree/overlay files to keep track of.  And barebox has to
> figure out which one to load for the kernel.

I think we compile in multiple overlays into barebox and let some board
code decide which of them to load. At least that's the plan. If that
really works good enough, we'll see. Currently I'm undecided whether
device tree overlay make life better or even more complicated.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

end of thread, other threads:[~2016-01-07  7:34 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-19  0:13 [PATCH 1/2] of: Add for_each_compatible_node_from iterator Trent Piepho
2015-12-19  0:18 ` [PATCH 2/2] of: Add of_property_for_each_phandle() iterator Trent Piepho
2016-01-04  8:32 ` [PATCH 1/2] of: Add for_each_compatible_node_from iterator Sascha Hauer
2016-01-04 19:01   ` [PATCH 1/2] OF: fix typo in for_each_compatible_node() Trent Piepho
2016-01-05  7:49     ` Sascha Hauer
2016-01-04 19:02   ` [PATCH 2/2] OF: Fix fixups to fix Linux DT instead of Barebox DT Trent Piepho
2016-01-05  7:50     ` Sascha Hauer
2016-01-04 19:07   ` [PATCH 1/2] of: Add for_each_compatible_node_from iterator Trent Piepho
2016-01-05  7:58     ` Sascha Hauer
2016-01-05  8:05       ` Yegor Yefremov
2016-01-05  8:20         ` Sascha Hauer
2016-01-05  8:30           ` Yegor Yefremov
2016-01-05 18:58       ` Trent Piepho
2016-01-07  7:34         ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox