Merging seperate FDT-blobs?

Merging seperate FDT-blobs?

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 940 bytes --]

Hello,

a project I am working on consists of different hardware modules, which
can be combined in a lot of variations (not at runtime, though). As each
module shall contain an I2C-eeprom, the idea is now to put a fragment of
a FDT-blob into that EEPROM and let the bootloader combine these
fragments. Such an approach was also sketched by David Gibson recently.
(http://ozlabs.org/pipermail/linuxppc-dev/2008-February/051630.html)

(As a sidenote, I am not quite sure and could not find anything about
what the mentioned 'fdt_graft' should actually do, in my case something
like a 'fdt_add_blob_subnode' function would be sufficent, I guess.)

My question: Is there already an effort towards such an approach to
which I could contribute, or do I have to start from scratch?

Kind regards,

   Wolfram

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Merging seperate FDT-blobs?

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1303 bytes --]

On Thu, Jun 12, 2008 at 11:04:33AM +0200, Wolfram Sang wrote:
> Hello,
> 
> a project I am working on consists of different hardware modules, which
> can be combined in a lot of variations (not at runtime, though). As each
> module shall contain an I2C-eeprom, the idea is now to put a fragment of
> a FDT-blob into that EEPROM and let the bootloader combine these
> fragments. Such an approach was also sketched by David Gibson recently.
> (http://ozlabs.org/pipermail/linuxppc-dev/2008-February/051630.html)
> 
> (As a sidenote, I am not quite sure and could not find anything about
> what the mentioned 'fdt_graft' should actually do, in my case something
> like a 'fdt_add_blob_subnode' function would be sufficent, I guess.)

fdt_add_blob_subnode() sounds like it would do the same thing as I
envisaged fdt_graft() doing.

> My question: Is there already an effort towards such an approach to
> which I could contribute, or do I have to start from scratch?

I don't know of anyone who's implemented this specifically, but if you
do get there first, please do it on top of libfdt.

-- 
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 #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Merging seperate FDT-blobs?

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 4840 bytes --]

On Fri, Jun 13, 2008 at 02:23:04PM +1000, David Gibson wrote:

> > a project I am working on consists of different hardware modules, which
> > can be combined in a lot of variations (not at runtime, though). As each
> > module shall contain an I2C-eeprom, the idea is now to put a fragment of
> > a FDT-blob into that EEPROM and let the bootloader combine these
> > fragments. Such an approach was also sketched by David Gibson recently.
[...]
> > My question: Is there already an effort towards such an approach to
> > which I could contribute, or do I have to start from scratch?
> 
> I don't know of anyone who's implemented this specifically, but if you
> do get there first, please do it on top of libfdt.

Yes, I would surely put this on top of libfdt.

Similar to what Stephen Neuendorffer wrote recently
(http://ozlabs.org/pipermail/linuxppc-dev/2008-June/058074.html), I'd
like to develop a solution which makes the fragments compilable
independently from a main tree. This would allow to store a fragment
on an extension board once and connect it to different kinds of baseboards
without any reconfiguration necessary.

So far, main problems I faced for such an approach are:

1) How do you know where a node from the fragment should be added to
   in the main tree?
   Imagine your extension board carries an I2C device, then you need to
   find the correct I2C bus which may have a different name on different
   baseboards (and there may be more than one bus).

2) How to deal with phandles?
   The fragment does not know anything about phandles from the main
   tree, so it cannot use them, although it needs to for a number of
   typical fdt-entries.

My proposal looks like the following (this is all still brainstorming
phase, so please feel free to join):

Create a property "external-name". This defines the name of the current
node regarding access from external fragments and must be unique.
Example for an i2c bus in the main tree:

	i2c@3d00 {
		#address-cells = <1>;
		#size-cells = <0>;
		compatible = "fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c";
		[...]
		external-name = "i2c-0";
	};

If you are strict on keeping consistent names for equal busses on
different baseboards, then this should ensure hardware independency.
Now a fragment, or an external, could look like this:

/ {
	// Using the external-name now.
	i2c-0 {
		eeprom@52 {
			device_type = "i2c";
			compatible = "linux,24c32";
			reg = <0x52>;
		};
	};
};

A boot-loader could add this node with an algorithm like the
following pseudo-code:

foreach node from external_fdt with depth == 1 {

	name = fdt_get_name(external_fdt, node);

	offset = fdt_node_offset_by_prop_value(main_fdt, "external-name", name);
	if (offset == -FDT_ERR_NOTFOUND)
		offset = fdt_add_subnode(main_fdt, 0 /* root level*/, name);

	copy_properties_and_subnodes(...);
}

So, it will take the name of each node with depth == 1 from the
external, look if there is an "external-name" set to the same name in
the main tree and add the node to the place found. If not found, it will
be put below the root-node. (Note that copy_properties_and_subnotes is
still an obstacle and has to be implemented)

The property "external-name" could also be used to deal with phandles.
In an external, fragment, or however you call it, properties requiring a
phandle from the main-fdt could be prefixed. Instead of:

	interrupt-parent = <&mpic>

we would write

	phandle-for-interrupt-parent = "mpic-0";

(maybe the prefix 'phandle-for-' could be replaced by a token?),
assuming there is a pic which has the property
'external-name = "mpic-0"'.

Then we have the following pseudocode for the bootloader, which would
find out the phandle for mpic-0 and set 'interrupt-parent' accordingly:

	if (property starts with "phandle-for-") {
		propname = property without 'phandle-for-';

		name = fdt_getprop(external_fdt, cur_node, property);

		phandle_offset = fdt_node_offset_by_prop_value(main_fdt, "external-name", name)
		phandle = fdt_get_phandle(main_fdt, phandle_offset);

		fdt_setprop(external_fdt, propname, phandle);
		copy_prop_to_main_fdt();
	}

Finally, the main tree needs to know where to look for externals.
Something like this should do:

	externals {
		blob0 {
			device-handle = <&eeprom>;
			offset = <0xf00>;
		};
	};

This can easily be parsed by a bootloader and should be flexible enough
to allow different kinds of memory.


This is what I have come up with so far. Still, my proposal is very
likely far from complete, so I am really looking forward to comments,
crticism or improvements.

Kind regards,

   Wolfram

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Merging seperate FDT-blobs?

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 8080 bytes --]

On Fri, Jun 20, 2008 at 06:10:33PM +0200, Wolfram Sang wrote:
> On Fri, Jun 13, 2008 at 02:23:04PM +1000, David Gibson wrote:
> 
> > > a project I am working on consists of different hardware modules, which
> > > can be combined in a lot of variations (not at runtime, though). As each
> > > module shall contain an I2C-eeprom, the idea is now to put a fragment of
> > > a FDT-blob into that EEPROM and let the bootloader combine these
> > > fragments. Such an approach was also sketched by David Gibson recently.
> [...]
> > > My question: Is there already an effort towards such an approach to
> > > which I could contribute, or do I have to start from scratch?
> > 
> > I don't know of anyone who's implemented this specifically, but if you
> > do get there first, please do it on top of libfdt.
> 
> Yes, I would surely put this on top of libfdt.
> 
> Similar to what Stephen Neuendorffer wrote recently
> (http://ozlabs.org/pipermail/linuxppc-dev/2008-June/058074.html), I'd
> like to develop a solution which makes the fragments compilable
> independently from a main tree. This would allow to store a fragment
> on an extension board once and connect it to different kinds of baseboards
> without any reconfiguration necessary.
> 
> So far, main problems I faced for such an approach are:
> 
> 1) How do you know where a node from the fragment should be added to
>    in the main tree?
>    Imagine your extension board carries an I2C device, then you need to
>    find the correct I2C bus which may have a different name on different
>    baseboards (and there may be more than one bus).

Hrm.  I was assuming this would be handled by the code putting things
together, rather than being encoded into the fragments.  I envisaged
something like:

	int fdt_graft(void *fdt, int parentoffset, const char *name,
		      void *fragment);

This would attach the tree fragment in the blob at 'fragment' into the
tree 'fdt', with fragment's root gaining the name 'name' and being a
child of the existing node at 'parentoffset'.

> 2) How to deal with phandles?
>    The fragment does not know anything about phandles from the main
>    tree, so it cannot use them, although it needs to for a number of
>    typical fdt-entries.

Ah.  This is harder.  And there are two sub-pieces to this problem.
First, how to ensure that any phandles for nodes in the fragment don't
collide with phandles from the parent tree or other included
fragments.  Second, how to fixup phandle references in between
initially separate fragments.

As an asside, I think going through the grafted fragment and adjusting
phandles to avoid collisions is a no-goer.  Any new binding can
potentially introduce a new property in which phandles appear, so we
can't really know where to look for phandles and adjust.

So, for the first part, for now, I think the sanest way to handle this
for now is simply to require that the fragments have non-overlapping
phandles.  For example by specifically allocating different ranges for
different fragments when assembling a collection of such things.
There are problems with this approach, and we may need something
better, but I think it's where we need to start.

At the moment that will require manually assigning all phandles, but
we could extend dtc with a directive giving the range in which to
assign phandles.

> My proposal looks like the following (this is all still brainstorming
> phase, so please feel free to join):
> 
> Create a property "external-name". This defines the name of the current
> node regarding access from external fragments and must be unique.
> Example for an i2c bus in the main tree:

Yeah, I don't really think this is a good idea.  Better to have the
assembly code handle this as suggested above.

> 
> 	i2c@3d00 {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		compatible = "fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c";
> 		[...]
> 		external-name = "i2c-0";
> 	};
> 
> If you are strict on keeping consistent names for equal busses on
> different baseboards, then this should ensure hardware independency.
> Now a fragment, or an external, could look like this:
> 
> / {
> 	// Using the external-name now.
> 	i2c-0 {
> 		eeprom@52 {
> 			device_type = "i2c";
> 			compatible = "linux,24c32";
> 			reg = <0x52>;
> 		};
> 	};
> };
> 
> A boot-loader could add this node with an algorithm like the
> following pseudo-code:
> 
> foreach node from external_fdt with depth == 1 {
> 
> 	name = fdt_get_name(external_fdt, node);
> 
> 	offset = fdt_node_offset_by_prop_value(main_fdt, "external-name", name);
> 	if (offset == -FDT_ERR_NOTFOUND)
> 		offset = fdt_add_subnode(main_fdt, 0 /* root level*/, name);
> 
> 	copy_properties_and_subnodes(...);
> }
> 
> So, it will take the name of each node with depth == 1 from the
> external, look if there is an "external-name" set to the same name in
> the main tree and add the node to the place found. If not found, it will
> be put below the root-node. (Note that copy_properties_and_subnotes is
> still an obstacle and has to be implemented)

Hrm.  I don't really follow the point of this scheme.  It seems like
you're encoding how to assemble the fragments into the master
fragment.  But the whole point of assembling fragments is that the
master can encode several different trees depending what's included,
which means something else needs to decide whether they're merged or
not.  I think just provide fdt_graft() as above, and leave the
decisions about where and whether to the assembly code in the wrapper
or bootloader.

> The property "external-name" could also be used to deal with phandles.
> In an external, fragment, or however you call it, properties requiring a
> phandle from the main-fdt could be prefixed. Instead of:
> 
> 	interrupt-parent = <&mpic>
> 
> we would write
> 
> 	phandle-for-interrupt-parent = "mpic-0";
> 
> (maybe the prefix 'phandle-for-' could be replaced by a token?),
> assuming there is a pic which has the property
> 'external-name = "mpic-0"'.

This doesn't work for properties which include phandles but are not
simply a single phandle; 'interrupt-map' for example.

> Then we have the following pseudocode for the bootloader, which would
> find out the phandle for mpic-0 and set 'interrupt-parent' accordingly:
> 
> 	if (property starts with "phandle-for-") {

We'll need property iteration for this, which we don't currently have.
But I've been intending to implement that for a while anyway.

> 		propname = property without 'phandle-for-';
> 
> 		name = fdt_getprop(external_fdt, cur_node, property);
> 
> 		phandle_offset = fdt_node_offset_by_prop_value(main_fdt, "external-name", name)
> 		phandle = fdt_get_phandle(main_fdt, phandle_offset);
> 
> 		fdt_setprop(external_fdt, propname, phandle);
> 		copy_prop_to_main_fdt();
> 	}

Hrm.  This doesn't seem all that useful.  It only handles adjusting
phandle references that are in the external tree pointing into one of
the fragments.  Since the fragments more-or-less by definition aren't
always present, this is an unlikely use case.  What I *can* see
happening is things within the fragments wanting to refer to an
always-present piece of hardware outside - the main interrupt
controller, in particular.

> Finally, the main tree needs to know where to look for externals.
> Something like this should do:
> 
> 	externals {
> 		blob0 {
> 			device-handle = <&eeprom>;
> 			offset = <0xf00>;
> 		};
> 	};
> 
> This can easily be parsed by a bootloader and should be flexible enough
> to allow different kinds of memory.
> 
> 
> This is what I have come up with so far. Still, my proposal is very
> likely far from complete, so I am really looking forward to comments,
> crticism or improvements.

-- 
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 #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Merging seperate FDT-blobs?

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 6565 bytes --]

Hello David,

On Mon, Jun 23, 2008 at 03:06:24PM +1000, David Gibson wrote:

> Hrm.  I was assuming this would be handled by the code putting things
> together, rather than being encoded into the fragments.  I envisaged
> something like:
> 
> 	int fdt_graft(void *fdt, int parentoffset, const char *name,
> 		      void *fragment);
> 
> This would attach the tree fragment in the blob at 'fragment' into the
> tree 'fdt', with fragment's root gaining the name 'name' and being a
> child of the existing node at 'parentoffset'.

This function is surely needed in every case I considered so far. I am
just sceptical if the boot-loader can determine a correct parentoffset
all alone (which one of the two I2C busses is the correct one?). This is
why I added the property "external-name" as a helping hand. Meanwhile, I
found your proposal about handling the aliases-node
(http://ozlabs.org/pipermail/linuxppc-dev/2007-November/045778.html) 
and think it will be a better alternative (as mentioned below).

> > 2) How to deal with phandles?
> >    The fragment does not know anything about phandles from the main
> >    tree, so it cannot use them, although it needs to for a number of
> >    typical fdt-entries.
> 
> Ah.  This is harder.  And there are two sub-pieces to this problem.
> First, how to ensure that any phandles for nodes in the fragment don't
> collide with phandles from the parent tree or other included
> fragments.  Second, how to fixup phandle references in between
> initially separate fragments.

> As an asside, I think going through the grafted fragment and adjusting
> phandles to avoid collisions is a no-goer.  Any new binding can
> potentially introduce a new property in which phandles appear, so we
> can't really know where to look for phandles and adjust.

True. I didn't take into account that phandles may show up anywhere,
even in a field of cells. Sigh, I wish phandles had some marker tag
(no, I don't expect this to happen).

> So, for the first part, for now, I think the sanest way to handle this
> for now is simply to require that the fragments have non-overlapping
> phandles.  For example by specifically allocating different ranges for
> different fragments when assembling a collection of such things.
> There are problems with this approach, and we may need something
> better, but I think it's where we need to start.

> At the moment that will require manually assigning all phandles, but
> we could extend dtc with a directive giving the range in which to
> assign phandles.

So, if dtc has the possibility to specify a range for phandles, every
fragment gets its own "namespace" for phandles. Handles which are
clearly not leaving the namespace can still be referenced using the
convenient <&phandle> syntax. Every phandle which needs to be accessed
from different namespaces, has to be hardcoded, preferably in their own
hardcoded namespace (e.g. linux,phandle = <0xfffffxyz>). Nice thing is,
that it still can be assigned a dtc-label, so referencing inside the
same source is also easy to maintain. Using macro features, one could
even use #defines for the hardcoded values.

Still, I sense some practical issues. I guess I need to sketch some
more possible constellations to get an even bigger picture. Problem is
as often: if you get generic, you get complexity.

> Hrm.  I don't really follow the point of this scheme.  It seems like
> you're encoding how to assemble the fragments into the master
> fragment.

What I encoded using "external-name" is where possible fragments
_could_ be added to. Something like a mount-point. The boot-loader
decides if and what could be mounted there. As an "/aliases" node is
already in use, I would favour to add such mount-points there.

> But the whole point of assembling fragments is that the
> master can encode several different trees depending what's included,
> which means something else needs to decide whether they're merged or
> not.  I think just provide fdt_graft() as above, and leave the
> decisions about where and whether to the assembly code in the wrapper
> or bootloader.

I assume the main tagret for this criticism is the "externals"-node I
introduced at the very end. I agree that its functionality could be
transferred to the boot-loader (e.g. give the boot-loader a list of
addresses to check for potential eeproms). I wouldn't mind dropping the
"externals" node; sounds in deed a bit cleaner.

> This doesn't work for properties which include phandles but are not
> simply a single phandle; 'interrupt-map' for example.

Check mate :(

> We'll need property iteration for this, which we don't currently have.
> But I've been intending to implement that for a while anyway.

I am quite sure it will come handy on the way nontheless.

> 
> > 		propname = property without 'phandle-for-';
> > 
> > 		name = fdt_getprop(external_fdt, cur_node, property);
> > 
> > 		phandle_offset = fdt_node_offset_by_prop_value(main_fdt, "external-name", name)
> > 		phandle = fdt_get_phandle(main_fdt, phandle_offset);
> > 
> > 		fdt_setprop(external_fdt, propname, phandle);
> > 		copy_prop_to_main_fdt();
> > 	}
> 
> Hrm.  This doesn't seem all that useful.  It only handles adjusting
> phandle references that are in the external tree pointing into one of
> the fragments.  Since the fragments more-or-less by definition aren't
> always present, this is an unlikely use case.  What I *can* see
> happening is things within the fragments wanting to refer to an
> always-present piece of hardware outside - the main interrupt
> controller, in particular.
> 

Ehrm, most likely my example was not clear enough, I am sorry. The case
of a fragment wanting to reach the main interrupt controller was exactly
what I had in mind when writing this. For completeness, I will try to
clear it up, although it won't work, as stated before: The
'phandle-for-*' property is in the fragment. It's value (let's say
'pic-0') is taken from the fragment. An apropriate mount-point (set by
'external-name = "pic-0"' in the main tree) is searched in the main
tree.  The phandle of the mount point is taked from the main tree. This
value will become the new phandle inside the fragment pointing to the
main tree. In the end, the whole fragment will be grafted into the main
tree, of course.

Thanks for your feedback!

   Wolfram

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Merging seperate FDT-blobs?

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 7506 bytes --]

On Fri, Jun 27, 2008 at 07:05:47PM +0200, Wolfram Sang wrote:
> Hello David,
> 
> On Mon, Jun 23, 2008 at 03:06:24PM +1000, David Gibson wrote:
> 
> > Hrm.  I was assuming this would be handled by the code putting things
> > together, rather than being encoded into the fragments.  I envisaged
> > something like:
> > 
> > 	int fdt_graft(void *fdt, int parentoffset, const char *name,
> > 		      void *fragment);
> > 
> > This would attach the tree fragment in the blob at 'fragment' into the
> > tree 'fdt', with fragment's root gaining the name 'name' and being a
> > child of the existing node at 'parentoffset'.
> 
> This function is surely needed in every case I considered so far. I am
> just sceptical if the boot-loader can determine a correct parentoffset
> all alone (which one of the two I2C busses is the correct one?). This is

Hrm.  "all alone".  It's not clear to me what else there could be that
would have more information than the bootloader.

> why I added the property "external-name" as a helping hand. Meanwhile, I
> found your proposal about handling the aliases-node
> (http://ozlabs.org/pipermail/linuxppc-dev/2007-November/045778.html) 
> and think it will be a better alternative (as mentioned below).
> 
> > > 2) How to deal with phandles?
> > >    The fragment does not know anything about phandles from the main
> > >    tree, so it cannot use them, although it needs to for a number of
> > >    typical fdt-entries.
> > 
> > Ah.  This is harder.  And there are two sub-pieces to this problem.
> > First, how to ensure that any phandles for nodes in the fragment don't
> > collide with phandles from the parent tree or other included
> > fragments.  Second, how to fixup phandle references in between
> > initially separate fragments.
> 
> > As an asside, I think going through the grafted fragment and adjusting
> > phandles to avoid collisions is a no-goer.  Any new binding can
> > potentially introduce a new property in which phandles appear, so we
> > can't really know where to look for phandles and adjust.
> 
> True. I didn't take into account that phandles may show up anywhere,
> even in a field of cells. Sigh, I wish phandles had some marker tag
> (no, I don't expect this to happen).
> 
> > So, for the first part, for now, I think the sanest way to handle this
> > for now is simply to require that the fragments have non-overlapping
> > phandles.  For example by specifically allocating different ranges for
> > different fragments when assembling a collection of such things.
> > There are problems with this approach, and we may need something
> > better, but I think it's where we need to start.
> 
> > At the moment that will require manually assigning all phandles, but
> > we could extend dtc with a directive giving the range in which to
> > assign phandles.
> 
> So, if dtc has the possibility to specify a range for phandles, every
> fragment gets its own "namespace" for phandles. Handles which are
> clearly not leaving the namespace can still be referenced using the
> convenient <&phandle> syntax. Every phandle which needs to be accessed
> from different namespaces, has to be hardcoded, preferably in their own
> hardcoded namespace (e.g. linux,phandle = <0xfffffxyz>). Nice thing is,
> that it still can be assigned a dtc-label, so referencing inside the
> same source is also easy to maintain. Using macro features, one could
> even use #defines for the hardcoded values.

Exactly.  It's not perfect, but I think it's an ok compromise of
convenience of use and simplicity of implementation.  At least as a
first step.

> Still, I sense some practical issues. I guess I need to sketch some
> more possible constellations to get an even bigger picture. Problem is
> as often: if you get generic, you get complexity.
> 
> > Hrm.  I don't really follow the point of this scheme.  It seems like
> > you're encoding how to assemble the fragments into the master
> > fragment.
> 
> What I encoded using "external-name" is where possible fragments
> _could_ be added to. Something like a mount-point. The boot-loader
> decides if and what could be mounted there. As an "/aliases" node is
> already in use, I would favour to add such mount-points there.

Hrm.  I'm not convinced that the mount point model is actually a good
one.  I would have thought that one of the most common things to graft
would be extra optional devices onto a bus.  In this case there's no
specific "mountpoint"  the device could be attached at any valid
address on the bus in question.

> > But the whole point of assembling fragments is that the
> > master can encode several different trees depending what's included,
> > which means something else needs to decide whether they're merged or
> > not.  I think just provide fdt_graft() as above, and leave the
> > decisions about where and whether to the assembly code in the wrapper
> > or bootloader.
> 
> I assume the main tagret for this criticism is the "externals"-node I
> introduced at the very end. I agree that its functionality could be
> transferred to the boot-loader (e.g. give the boot-loader a list of
> addresses to check for potential eeproms). I wouldn't mind dropping the
> "externals" node; sounds in deed a bit cleaner.
> 
> > This doesn't work for properties which include phandles but are not
> > simply a single phandle; 'interrupt-map' for example.
> 
> Check mate :(
> 
> > We'll need property iteration for this, which we don't currently have.
> > But I've been intending to implement that for a while anyway.
> 
> I am quite sure it will come handy on the way nontheless.
> 
> > 
> > > 		propname = property without 'phandle-for-';
> > > 
> > > 		name = fdt_getprop(external_fdt, cur_node, property);
> > > 
> > > 		phandle_offset = fdt_node_offset_by_prop_value(main_fdt, "external-name", name)
> > > 		phandle = fdt_get_phandle(main_fdt, phandle_offset);
> > > 
> > > 		fdt_setprop(external_fdt, propname, phandle);
> > > 		copy_prop_to_main_fdt();
> > > 	}
> > 
> > Hrm.  This doesn't seem all that useful.  It only handles adjusting
> > phandle references that are in the external tree pointing into one of
> > the fragments.  Since the fragments more-or-less by definition aren't
> > always present, this is an unlikely use case.  What I *can* see
> > happening is things within the fragments wanting to refer to an
> > always-present piece of hardware outside - the main interrupt
> > controller, in particular.
> 
> Ehrm, most likely my example was not clear enough, I am sorry. The case
> of a fragment wanting to reach the main interrupt controller was exactly
> what I had in mind when writing this. For completeness, I will try to
> clear it up, although it won't work, as stated before: The
> 'phandle-for-*' property is in the fragment. It's value (let's say
> 'pic-0') is taken from the fragment. An apropriate mount-point (set by
> 'external-name = "pic-0"' in the main tree) is searched in the main
> tree.  The phandle of the mount point is taked from the main tree. This
> value will become the new phandle inside the fragment pointing to the
> main tree. In the end, the whole fragment will be grafted into the main
> tree, of course.

-- 
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 #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Merging seperate FDT-blobs?

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2156 bytes --]

On Mon, Jul 07, 2008 at 11:28:43AM +1000, David Gibson wrote:

> > This function is surely needed in every case I considered so far. I am
> > just sceptical if the boot-loader can determine a correct parentoffset
> > all alone (which one of the two I2C busses is the correct one?). This is
> 
> Hrm.  "all alone".  It's not clear to me what else there could be that
> would have more information than the bootloader.

What I meant is that all the information a bootloader has may not be
sufficent. To solve this, some additional infos could be added to the
tree. In this case, it could be a few aliases.

> > What I encoded using "external-name" is where possible fragments
> > _could_ be added to. Something like a mount-point. The boot-loader
> > decides if and what could be mounted there. As an "/aliases" node is
> > already in use, I would favour to add such mount-points there.
> 
> Hrm.  I'm not convinced that the mount point model is actually a good
> one.  I would have thought that one of the most common things to graft
> would be extra optional devices onto a bus.  In this case there's no
> specific "mountpoint"  the device could be attached at any valid
> address on the bus in question.

Maybe I am really missing something here, but what is the bus in
question? How do you tell from such an entry

rtc@51{
	device_type="rtc";
	compatible="nxp,pcf8563";
	reg=<0x51>;
};

if it is connected to "/pci/pci_bridge/isa" as in mpc8548cds.dts or to
"/soc5200/i2c@3d40" as in pcm030.dts? The latter even has another
I2C-bus i2c@3d00 which could also be a possibility. This is why I'd like
to encode the fragment as:

i2c_0 {
	rtc@51{
		device_type="rtc";
		compatible="nxp,pcf8563";
		reg=<0x51>;
	};
};

with i2c_0 being an alias to the proper bus. Maybe I should add that I
am _not_ assuming that the fragment is obtained from the bus which wants to
have devices added. That is, one I2C-eeprom may contain data about
additional devices on PCI.

Kind regards,

   Wolfram

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Merging seperate FDT-blobs?

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]

On Mon, Jul 07, 2008 at 11:23:47AM +0200, Wolfram Sang wrote:
> On Mon, Jul 07, 2008 at 11:28:43AM +1000, David Gibson wrote:
> 
> > > This function is surely needed in every case I considered so far. I am
> > > just sceptical if the boot-loader can determine a correct parentoffset
> > > all alone (which one of the two I2C busses is the correct one?). This is
> > 
> > Hrm.  "all alone".  It's not clear to me what else there could be that
> > would have more information than the bootloader.
> 
> What I meant is that all the information a bootloader has may not be
> sufficent. To solve this, some additional infos could be added to the
> tree. In this case, it could be a few aliases.

My point is that the dts and the bootloader will typically be built by
the same person as part of the same process.  What is the rationale
for encoding this information into the tree fragments rather than the
code?

-- 
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 #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]