diff mbox

[U-Boot,3/5] fdt: Allow stacked overlays phandle references

Message ID 1498839782-4702-4-git-send-email-pantelis.antoniou@konsulko.com
State Deferred
Delegated to: Simon Glass
Headers show

Commit Message

Pantelis Antoniou June 30, 2017, 4:23 p.m. UTC
This patch enables an overlay to refer to a previous overlay's
labels by performing a merge of symbol information at application
time.

In a nutshell it allows an overlay to refer to a symbol that a previous
overlay has defined. It requires both the base and all the overlays
to be compiled with the -@ command line switch so that symbol
information is included.

base.dts
--------

	/dts-v1/;
	/ {
		foo: foonode {
			foo-property;
		};
	};

	$ dtc -@ -I dts -O dtb -o base.dtb base.dts

bar.dts
-------

	/dts-v1/;
	/plugin/;
	/ {
		fragment@1 {
			target = <&foo>;
			__overlay__ {
				overlay-1-property;
				bar: barnode {
					bar-property;
				};
			};
		};
	};

	$ dtc -@ -I dts -O dtb -o bar.dtb bar.dts

baz.dts
-------

	/dts-v1/;
	/plugin/;
	/ {
		fragment@1 {
			target = <&bar>;
			__overlay__ {
				overlay-2-property;
				baz: baznode {
					baz-property;
				};
			};
		};
	};

	$ dtc -@ -I dts -O dtb -o baz.dtb baz.dts

Applying the overlays:

	$ fdtoverlay -i base.dtb -o target.dtb bar.dtb baz.dtb

Dumping:

	$ fdtdump target.dtb
	/ {
            foonode {
                overlay-1-property;
                foo-property;
                linux,phandle = <0x00000001>;
                phandle = <0x00000001>;
                barnode {
                    overlay-2-property;
                    phandle = <0x00000002>;
                    linux,phandle = <0x00000002>;
                    bar-property;
                    baznode {
                        phandle = <0x00000003>;
                        linux,phandle = <0x00000003>;
                        baz-property;
                    };
                };
            };
            __symbols__ {
                baz = "/foonode/barnode/baznode";
                bar = "/foonode/barnode";
                foo = "/foonode";
            };
	};

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 lib/libfdt/fdt_overlay.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 147 insertions(+), 1 deletion(-)

Comments

Marek Vasut July 1, 2017, 2:07 p.m. UTC | #1
On 06/30/2017 06:23 PM, Pantelis Antoniou wrote:

[...]

> +static int overlay_symbol_update(void *fdt, void *fdto)
> +{
> +	int root_sym, ov_sym, prop, path_len, fragment, target;
> +	int len, frag_name_len, ret, rel_path_len;
> +	const char *s;
> +	const char *path;
> +	const char *name;
> +	const char *frag_name;
> +	const char *rel_path;
> +	char *buf = NULL;
> +
> +	root_sym = fdt_subnode_offset(fdt, 0, "__symbols__");
> +	ov_sym = fdt_subnode_offset(fdto, 0, "__symbols__");
> +
> +	/* if neither exist we can't update symbols, but that's OK */
> +	if (root_sym < 0 || ov_sym < 0)
> +		return 0;

If you have symbol table in either the DTO or the base DT, but not in
the other one, wouldn't it make sense to use that one symbol table
instead of bailing out ?

> +	buf = malloc(FDT_PATH_MAX);
> +	if (!buf)
> +		return -FDT_ERR_NOSPACE;

Would it make sense to allocate this on stack ?

> +	/* iterate over each overlay symbol */
> +	fdt_for_each_property_offset(prop, fdto, ov_sym) {
> +
> +		path = fdt_getprop_by_offset(fdto, prop, &name, &path_len);
> +		if (!path) {
> +			ret = path_len;
> +			goto out;
> +		}
> +
> +		/* skip autogenerated properties */
> +		if (!strcmp(name, "name"))
> +			continue;
> +
> +		/* format: /<fragment-name>/__overlay__/<relative-subnode-path> */
> +
> +		if (*path != '/') {
> +			ret = -FDT_ERR_BADVALUE;
> +			goto out;
> +		}
> +
> +		/* get frag name first */

"frag name" ? What is this now, Unreal Tournament ? :)

> +		s = strchr(path + 1, '/');
> +		if (!s) {
> +			ret = -FDT_ERR_BADVALUE;
> +			goto out;
> +		}
> +		frag_name = path + 1;
> +		frag_name_len = s - path - 1;
> +
> +		/* verify format */
> +		len = strlen("/__overlay__/");
> +		if (strncmp(s, "/__overlay__/", len)) {
> +			ret = -FDT_ERR_NOTFOUND;
> +			goto out;
> +		}
> +
> +		rel_path = s + len;
> +		rel_path_len = strlen(rel_path);
> +
> +		/* find the fragment index in which the symbol lies */
> +		fdt_for_each_subnode(fragment, fdto, 0) {
> +
> +			s = fdt_get_name(fdto, fragment, &len);
> +			if (!s)
> +				continue;
> +
> +			/* name must match */
> +			if (len == frag_name_len && !memcmp(s, frag_name, len))
> +				break;
> +		}
> +
> +		/* not found? */
> +		if (fragment < 0) {
> +			ret = -FDT_ERR_NOTFOUND;
> +			goto out;
> +		}
> +
> +		/* an __overlay__ subnode must exist */
> +		ret = fdt_subnode_offset(fdto, fragment, "__overlay__");
> +		if (ret < 0)
> +			goto out;
> +
> +		/* get the target of the fragment */
> +		ret = overlay_get_target(fdt, fdto, fragment);
> +		if (ret < 0)
> +			goto out;
> +		target = ret;
> +
> +		/* get the path of the target */
> +		ret = fdt_get_path(fdt, target, buf, FDT_PATH_MAX);
> +		if (ret < 0)
> +			goto out;
> +
> +		len = strlen(buf);
> +
> +		/* if the target is root strip leading / */
> +		if (len == 1 && buf[0] == '/')
> +			len = 0;
> +
> +		/* make sure we have enough space */
> +		if (len + 1 + rel_path_len + 1 > FDT_PATH_MAX) {
> +			ret = -FDT_ERR_NOSPACE;
> +			goto out;
> +		}
> +
> +		/* create new symbol path in place */
> +		buf[len] = '/';
> +		memcpy(buf + len + 1, rel_path, rel_path_len);
> +		buf[len + 1 + rel_path_len] = '\0';

Can snprintf() help somewhere here instead of the memcpy() ?

> +		ret = fdt_setprop_string(fdt, root_sym, name, buf);
> +		if (ret < 0)
> +			goto out;
> +	}
> +
> +	ret = 0;
> +
> +out:
> +	if (buf)
> +		free(buf);
> +
> +	return ret;
> +}
> +
>  int fdt_overlay_apply(void *fdt, void *fdto)
>  {
>  	uint32_t delta = fdt_get_max_phandle(fdt);
> @@ -654,6 +796,10 @@ int fdt_overlay_apply(void *fdt, void *fdto)
>  	if (ret)
>  		goto err;
>  
> +	ret = overlay_symbol_update(fdt, fdto);
> +	if (ret)
> +		goto err;
> +
>  	/*
>  	 * The overlay has been damaged, erase its magic.
>  	 */
>
Pantelis Antoniou July 4, 2017, 5:03 p.m. UTC | #2
Hi Marek,

On Sat, 2017-07-01 at 16:07 +0200, Marek Vasut wrote:
> On 06/30/2017 06:23 PM, Pantelis Antoniou wrote:
> 
> [...]
> 
> > +static int overlay_symbol_update(void *fdt, void *fdto)
> > +{
> > +	int root_sym, ov_sym, prop, path_len, fragment, target;
> > +	int len, frag_name_len, ret, rel_path_len;
> > +	const char *s;
> > +	const char *path;
> > +	const char *name;
> > +	const char *frag_name;
> > +	const char *rel_path;
> > +	char *buf = NULL;
> > +
> > +	root_sym = fdt_subnode_offset(fdt, 0, "__symbols__");
> > +	ov_sym = fdt_subnode_offset(fdto, 0, "__symbols__");
> > +
> > +	/* if neither exist we can't update symbols, but that's OK */
> > +	if (root_sym < 0 || ov_sym < 0)
> > +		return 0;
> 
> If you have symbol table in either the DTO or the base DT, but not in
> the other one, wouldn't it make sense to use that one symbol table
> instead of bailing out ?
> 

It's bailing out without an error.

The logic is simple; if there's no symbol table in the base tree, that
means that there's no symbols in the base device tree, i.e. not compiled
with the option to generate it. No point in inserting overlay symbols.

If there's no symbol table in the overlay that perfectly fine, no symbol
work then.

> > +	buf = malloc(FDT_PATH_MAX);
> > +	if (!buf)
> > +		return -FDT_ERR_NOSPACE;
> 
> Would it make sense to allocate this on stack ?
> 

FDT_PATH_MAX = 4K, dubious if the u-boot stack will have enough space.
Especially in SPL cases. 

> > +	/* iterate over each overlay symbol */
> > +	fdt_for_each_property_offset(prop, fdto, ov_sym) {
> > +
> > +		path = fdt_getprop_by_offset(fdto, prop, &name, &path_len);
> > +		if (!path) {
> > +			ret = path_len;
> > +			goto out;
> > +		}
> > +
> > +		/* skip autogenerated properties */
> > +		if (!strcmp(name, "name"))
> > +			continue;
> > +
> > +		/* format: /<fragment-name>/__overlay__/<relative-subnode-path> */
> > +
> > +		if (*path != '/') {
> > +			ret = -FDT_ERR_BADVALUE;
> > +			goto out;
> > +		}
> > +
> > +		/* get frag name first */
> 
> "frag name" ? What is this now, Unreal Tournament ? :)
> 

Fragment. There were computers before first person shooters.

> > +		s = strchr(path + 1, '/');
> > +		if (!s) {
> > +			ret = -FDT_ERR_BADVALUE;
> > +			goto out;
> > +		}
> > +		frag_name = path + 1;
> > +		frag_name_len = s - path - 1;
> > +
> > +		/* verify format */
> > +		len = strlen("/__overlay__/");
> > +		if (strncmp(s, "/__overlay__/", len)) {
> > +			ret = -FDT_ERR_NOTFOUND;
> > +			goto out;
> > +		}
> > +
> > +		rel_path = s + len;
> > +		rel_path_len = strlen(rel_path);
> > +
> > +		/* find the fragment index in which the symbol lies */
> > +		fdt_for_each_subnode(fragment, fdto, 0) {
> > +
> > +			s = fdt_get_name(fdto, fragment, &len);
> > +			if (!s)
> > +				continue;
> > +
> > +			/* name must match */
> > +			if (len == frag_name_len && !memcmp(s, frag_name, len))
> > +				break;
> > +		}
> > +
> > +		/* not found? */
> > +		if (fragment < 0) {
> > +			ret = -FDT_ERR_NOTFOUND;
> > +			goto out;
> > +		}
> > +
> > +		/* an __overlay__ subnode must exist */
> > +		ret = fdt_subnode_offset(fdto, fragment, "__overlay__");
> > +		if (ret < 0)
> > +			goto out;
> > +
> > +		/* get the target of the fragment */
> > +		ret = overlay_get_target(fdt, fdto, fragment);
> > +		if (ret < 0)
> > +			goto out;
> > +		target = ret;
> > +
> > +		/* get the path of the target */
> > +		ret = fdt_get_path(fdt, target, buf, FDT_PATH_MAX);
> > +		if (ret < 0)
> > +			goto out;
> > +
> > +		len = strlen(buf);
> > +
> > +		/* if the target is root strip leading / */
> > +		if (len == 1 && buf[0] == '/')
> > +			len = 0;
> > +
> > +		/* make sure we have enough space */
> > +		if (len + 1 + rel_path_len + 1 > FDT_PATH_MAX) {
> > +			ret = -FDT_ERR_NOSPACE;
> > +			goto out;
> > +		}
> > +
> > +		/* create new symbol path in place */
> > +		buf[len] = '/';
> > +		memcpy(buf + len + 1, rel_path, rel_path_len);
> > +		buf[len + 1 + rel_path_len] = '\0';
> 
> Can snprintf() help somewhere here instead of the memcpy() ?
> 

It is going to be considerably slower (although that doesn't matter
much). We still need to do the space check before so it's not going to
be worth the change IMO.
 
> > +		ret = fdt_setprop_string(fdt, root_sym, name, buf);
> > +		if (ret < 0)
> > +			goto out;
> > +	}
> > +
> > +	ret = 0;
> > +
> > +out:
> > +	if (buf)
> > +		free(buf);
> > +
> > +	return ret;
> > +}
> > +
> >  int fdt_overlay_apply(void *fdt, void *fdto)
> >  {
> >  	uint32_t delta = fdt_get_max_phandle(fdt);
> > @@ -654,6 +796,10 @@ int fdt_overlay_apply(void *fdt, void *fdto)
> >  	if (ret)
> >  		goto err;
> >  
> > +	ret = overlay_symbol_update(fdt, fdto);
> > +	if (ret)
> > +		goto err;
> > +
> >  	/*
> >  	 * The overlay has been damaged, erase its magic.
> >  	 */
> > 
> 
> 

Regards

-- Pantelis
Lothar Waßmann July 5, 2017, 6:25 a.m. UTC | #3
Hi,

On Sat, 1 Jul 2017 16:07:47 +0200 Marek Vasut wrote:
> On 06/30/2017 06:23 PM, Pantelis Antoniou wrote:
> 
[...]
> > +	buf = malloc(FDT_PATH_MAX);
> > +	if (!buf)
> > +		return -FDT_ERR_NOSPACE;
> 
> Would it make sense to allocate this on stack ?
> 
buffers on stack are a disatrous stack overflow waiting to happen.


Lothar Waßmann
Simon Glass July 7, 2017, 3:58 a.m. UTC | #4
On 30 June 2017 at 10:23, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:
> This patch enables an overlay to refer to a previous overlay's
> labels by performing a merge of symbol information at application
> time.
>
> In a nutshell it allows an overlay to refer to a symbol that a previous
> overlay has defined. It requires both the base and all the overlays
> to be compiled with the -@ command line switch so that symbol
> information is included.
>
> base.dts
> --------
>
>         /dts-v1/;
>         / {
>                 foo: foonode {
>                         foo-property;
>                 };
>         };
>
>         $ dtc -@ -I dts -O dtb -o base.dtb base.dts
>
> bar.dts
> -------
>
>         /dts-v1/;
>         /plugin/;
>         / {
>                 fragment@1 {
>                         target = <&foo>;
>                         __overlay__ {
>                                 overlay-1-property;
>                                 bar: barnode {
>                                         bar-property;
>                                 };
>                         };
>                 };
>         };
>
>         $ dtc -@ -I dts -O dtb -o bar.dtb bar.dts
>
> baz.dts
> -------
>
>         /dts-v1/;
>         /plugin/;
>         / {
>                 fragment@1 {
>                         target = <&bar>;
>                         __overlay__ {
>                                 overlay-2-property;
>                                 baz: baznode {
>                                         baz-property;
>                                 };
>                         };
>                 };
>         };
>
>         $ dtc -@ -I dts -O dtb -o baz.dtb baz.dts
>
> Applying the overlays:
>
>         $ fdtoverlay -i base.dtb -o target.dtb bar.dtb baz.dtb
>
> Dumping:
>
>         $ fdtdump target.dtb
>         / {
>             foonode {
>                 overlay-1-property;
>                 foo-property;
>                 linux,phandle = <0x00000001>;
>                 phandle = <0x00000001>;
>                 barnode {
>                     overlay-2-property;
>                     phandle = <0x00000002>;
>                     linux,phandle = <0x00000002>;
>                     bar-property;
>                     baznode {
>                         phandle = <0x00000003>;
>                         linux,phandle = <0x00000003>;
>                         baz-property;
>                     };
>                 };
>             };
>             __symbols__ {
>                 baz = "/foonode/barnode/baznode";
>                 bar = "/foonode/barnode";
>                 foo = "/foonode";
>             };
>         };
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> ---
>  lib/libfdt/fdt_overlay.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 147 insertions(+), 1 deletion(-)

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

I suppose that the upstream version has tests?

Does it make sense to implement this in the live tree instead, or do
you need to modify the DT before relocation?

- Simon
Pantelis Antoniou July 7, 2017, 7:02 a.m. UTC | #5
Hi Simon,

On Thu, 2017-07-06 at 21:58 -0600, Simon Glass wrote:
> On 30 June 2017 at 10:23, Pantelis Antoniou
> <pantelis.antoniou@konsulko.com> wrote:
> > This patch enables an overlay to refer to a previous overlay's
> > labels by performing a merge of symbol information at application
> > time.
> >
> > In a nutshell it allows an overlay to refer to a symbol that a previous
> > overlay has defined. It requires both the base and all the overlays
> > to be compiled with the -@ command line switch so that symbol
> > information is included.
> >
> > base.dts
> > --------
> >
> >         /dts-v1/;
> >         / {
> >                 foo: foonode {
> >                         foo-property;
> >                 };
> >         };
> >
> >         $ dtc -@ -I dts -O dtb -o base.dtb base.dts
> >
> > bar.dts
> > -------
> >
> >         /dts-v1/;
> >         /plugin/;
> >         / {
> >                 fragment@1 {
> >                         target = <&foo>;
> >                         __overlay__ {
> >                                 overlay-1-property;
> >                                 bar: barnode {
> >                                         bar-property;
> >                                 };
> >                         };
> >                 };
> >         };
> >
> >         $ dtc -@ -I dts -O dtb -o bar.dtb bar.dts
> >
> > baz.dts
> > -------
> >
> >         /dts-v1/;
> >         /plugin/;
> >         / {
> >                 fragment@1 {
> >                         target = <&bar>;
> >                         __overlay__ {
> >                                 overlay-2-property;
> >                                 baz: baznode {
> >                                         baz-property;
> >                                 };
> >                         };
> >                 };
> >         };
> >
> >         $ dtc -@ -I dts -O dtb -o baz.dtb baz.dts
> >
> > Applying the overlays:
> >
> >         $ fdtoverlay -i base.dtb -o target.dtb bar.dtb baz.dtb
> >
> > Dumping:
> >
> >         $ fdtdump target.dtb
> >         / {
> >             foonode {
> >                 overlay-1-property;
> >                 foo-property;
> >                 linux,phandle = <0x00000001>;
> >                 phandle = <0x00000001>;
> >                 barnode {
> >                     overlay-2-property;
> >                     phandle = <0x00000002>;
> >                     linux,phandle = <0x00000002>;
> >                     bar-property;
> >                     baznode {
> >                         phandle = <0x00000003>;
> >                         linux,phandle = <0x00000003>;
> >                         baz-property;
> >                     };
> >                 };
> >             };
> >             __symbols__ {
> >                 baz = "/foonode/barnode/baznode";
> >                 bar = "/foonode/barnode";
> >                 foo = "/foonode";
> >             };
> >         };
> >
> > Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> > ---
> >  lib/libfdt/fdt_overlay.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 147 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> I suppose that the upstream version has tests?
> 

Yes, and I could use a couple reviewers in the dtc mailing list.
 
> Does it make sense to implement this in the live tree instead, or do
> you need to modify the DT before relocation?
> 

There is a port of a patch that works on the live tree; I guess I can
port it to u-boot's live tree as well.

Speaking of which I've done quite a bit of work on a different live tree
implementation as well. I would propose we split off u-boot's one in a
library and port my changes there. That way we can have a definitive
live tree implementation, and in the future move dtc's internal live
tree to it as well. We keep on implementing the same functionality in
different flavors for years, let's consolidate, and start working on
ideas for the future.

> - Simon

Regards

-- Pantelis
Simon Glass July 14, 2017, 1:51 p.m. UTC | #6
Hi Pantelis,

On 7 July 2017 at 01:02, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:
> Hi Simon,
>
> On Thu, 2017-07-06 at 21:58 -0600, Simon Glass wrote:
>> On 30 June 2017 at 10:23, Pantelis Antoniou
>> <pantelis.antoniou@konsulko.com> wrote:
>> > This patch enables an overlay to refer to a previous overlay's
>> > labels by performing a merge of symbol information at application
>> > time.
>> >
>> > In a nutshell it allows an overlay to refer to a symbol that a previous
>> > overlay has defined. It requires both the base and all the overlays
>> > to be compiled with the -@ command line switch so that symbol
>> > information is included.
>> >
>> > base.dts
>> > --------
>> >
>> >         /dts-v1/;
>> >         / {
>> >                 foo: foonode {
>> >                         foo-property;
>> >                 };
>> >         };
>> >
>> >         $ dtc -@ -I dts -O dtb -o base.dtb base.dts
>> >
>> > bar.dts
>> > -------
>> >
>> >         /dts-v1/;
>> >         /plugin/;
>> >         / {
>> >                 fragment@1 {
>> >                         target = <&foo>;
>> >                         __overlay__ {
>> >                                 overlay-1-property;
>> >                                 bar: barnode {
>> >                                         bar-property;
>> >                                 };
>> >                         };
>> >                 };
>> >         };
>> >
>> >         $ dtc -@ -I dts -O dtb -o bar.dtb bar.dts
>> >
>> > baz.dts
>> > -------
>> >
>> >         /dts-v1/;
>> >         /plugin/;
>> >         / {
>> >                 fragment@1 {
>> >                         target = <&bar>;
>> >                         __overlay__ {
>> >                                 overlay-2-property;
>> >                                 baz: baznode {
>> >                                         baz-property;
>> >                                 };
>> >                         };
>> >                 };
>> >         };
>> >
>> >         $ dtc -@ -I dts -O dtb -o baz.dtb baz.dts
>> >
>> > Applying the overlays:
>> >
>> >         $ fdtoverlay -i base.dtb -o target.dtb bar.dtb baz.dtb
>> >
>> > Dumping:
>> >
>> >         $ fdtdump target.dtb
>> >         / {
>> >             foonode {
>> >                 overlay-1-property;
>> >                 foo-property;
>> >                 linux,phandle = <0x00000001>;
>> >                 phandle = <0x00000001>;
>> >                 barnode {
>> >                     overlay-2-property;
>> >                     phandle = <0x00000002>;
>> >                     linux,phandle = <0x00000002>;
>> >                     bar-property;
>> >                     baznode {
>> >                         phandle = <0x00000003>;
>> >                         linux,phandle = <0x00000003>;
>> >                         baz-property;
>> >                     };
>> >                 };
>> >             };
>> >             __symbols__ {
>> >                 baz = "/foonode/barnode/baznode";
>> >                 bar = "/foonode/barnode";
>> >                 foo = "/foonode";
>> >             };
>> >         };
>> >
>> > Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>> > ---
>> >  lib/libfdt/fdt_overlay.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++-
>> >  1 file changed, 147 insertions(+), 1 deletion(-)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> I suppose that the upstream version has tests?
>>
>
> Yes, and I could use a couple reviewers in the dtc mailing list.

Ah OK, I will take a look.

>
>> Does it make sense to implement this in the live tree instead, or do
>> you need to modify the DT before relocation?
>>
>
> There is a port of a patch that works on the live tree; I guess I can
> port it to u-boot's live tree as well.

Do you mean 'as well'? I am wondering if we can get away with only
supporting the live tree for overlays. Or do you think that will be
too limiting?

Obviously the live and flat tree versions do essentially the same
thing, so if we can it would be good to include only one version of
the code.

>
> Speaking of which I've done quite a bit of work on a different live tree
> implementation as well. I would propose we split off u-boot's one in a
> library and port my changes there. That way we can have a definitive
> live tree implementation, and in the future move dtc's internal live
> tree to it as well. We keep on implementing the same functionality in
> different flavors for years, let's consolidate, and start working on
> ideas for the future.

What about the kernel? I took most of the of_access.c code from there
and trying to avoid changing it as much as possible. The extra bits
for U-Boot are the ofnode implementation (to permit transparent use of
live/flat trees) and of course dev_read_...().

I think it would be great to have a live tree upstream in dtc that we
can pull into U-Boot. But I'm keen to line it up with Linux.

Regards,
Simon
diff mbox

Patch

diff --git a/lib/libfdt/fdt_overlay.c b/lib/libfdt/fdt_overlay.c
index ceb9687..59fd7f3 100644
--- a/lib/libfdt/fdt_overlay.c
+++ b/lib/libfdt/fdt_overlay.c
@@ -590,7 +590,7 @@  static int overlay_apply_node(void *fdt, int target,
  *
  * overlay_merge() merges an overlay into its base device tree.
  *
- * This is the final step in the device tree overlay application
+ * This is the next to last step in the device tree overlay application
  * process, when all the phandles have been adjusted and resolved and
  * you just have to merge overlay into the base device tree.
  *
@@ -630,6 +630,148 @@  static int overlay_merge(void *fdt, void *fdto)
 	return 0;
 }
 
+/**
+ * overlay_symbol_update - Update the symbols of base tree after a merge
+ * @fdt: Base Device Tree blob
+ * @fdto: Device tree overlay blob
+ *
+ * overlay_symbol_update() updates the symbols of the base tree with the
+ * symbols of the applied overlay
+ *
+ * This is the last step in the device tree overlay application
+ * process, allowing the reference of overlay symbols by subsequent
+ * overlay operations.
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_symbol_update(void *fdt, void *fdto)
+{
+	int root_sym, ov_sym, prop, path_len, fragment, target;
+	int len, frag_name_len, ret, rel_path_len;
+	const char *s;
+	const char *path;
+	const char *name;
+	const char *frag_name;
+	const char *rel_path;
+	char *buf = NULL;
+
+	root_sym = fdt_subnode_offset(fdt, 0, "__symbols__");
+	ov_sym = fdt_subnode_offset(fdto, 0, "__symbols__");
+
+	/* if neither exist we can't update symbols, but that's OK */
+	if (root_sym < 0 || ov_sym < 0)
+		return 0;
+
+	buf = malloc(FDT_PATH_MAX);
+	if (!buf)
+		return -FDT_ERR_NOSPACE;
+
+	/* iterate over each overlay symbol */
+	fdt_for_each_property_offset(prop, fdto, ov_sym) {
+
+		path = fdt_getprop_by_offset(fdto, prop, &name, &path_len);
+		if (!path) {
+			ret = path_len;
+			goto out;
+		}
+
+		/* skip autogenerated properties */
+		if (!strcmp(name, "name"))
+			continue;
+
+		/* format: /<fragment-name>/__overlay__/<relative-subnode-path> */
+
+		if (*path != '/') {
+			ret = -FDT_ERR_BADVALUE;
+			goto out;
+		}
+
+		/* get frag name first */
+		s = strchr(path + 1, '/');
+		if (!s) {
+			ret = -FDT_ERR_BADVALUE;
+			goto out;
+		}
+		frag_name = path + 1;
+		frag_name_len = s - path - 1;
+
+		/* verify format */
+		len = strlen("/__overlay__/");
+		if (strncmp(s, "/__overlay__/", len)) {
+			ret = -FDT_ERR_NOTFOUND;
+			goto out;
+		}
+
+		rel_path = s + len;
+		rel_path_len = strlen(rel_path);
+
+		/* find the fragment index in which the symbol lies */
+		fdt_for_each_subnode(fragment, fdto, 0) {
+
+			s = fdt_get_name(fdto, fragment, &len);
+			if (!s)
+				continue;
+
+			/* name must match */
+			if (len == frag_name_len && !memcmp(s, frag_name, len))
+				break;
+		}
+
+		/* not found? */
+		if (fragment < 0) {
+			ret = -FDT_ERR_NOTFOUND;
+			goto out;
+		}
+
+		/* an __overlay__ subnode must exist */
+		ret = fdt_subnode_offset(fdto, fragment, "__overlay__");
+		if (ret < 0)
+			goto out;
+
+		/* get the target of the fragment */
+		ret = overlay_get_target(fdt, fdto, fragment);
+		if (ret < 0)
+			goto out;
+		target = ret;
+
+		/* get the path of the target */
+		ret = fdt_get_path(fdt, target, buf, FDT_PATH_MAX);
+		if (ret < 0)
+			goto out;
+
+		len = strlen(buf);
+
+		/* if the target is root strip leading / */
+		if (len == 1 && buf[0] == '/')
+			len = 0;
+
+		/* make sure we have enough space */
+		if (len + 1 + rel_path_len + 1 > FDT_PATH_MAX) {
+			ret = -FDT_ERR_NOSPACE;
+			goto out;
+		}
+
+		/* create new symbol path in place */
+		buf[len] = '/';
+		memcpy(buf + len + 1, rel_path, rel_path_len);
+		buf[len + 1 + rel_path_len] = '\0';
+
+		ret = fdt_setprop_string(fdt, root_sym, name, buf);
+		if (ret < 0)
+			goto out;
+	}
+
+	ret = 0;
+
+out:
+	if (buf)
+		free(buf);
+
+	return ret;
+}
+
 int fdt_overlay_apply(void *fdt, void *fdto)
 {
 	uint32_t delta = fdt_get_max_phandle(fdt);
@@ -654,6 +796,10 @@  int fdt_overlay_apply(void *fdt, void *fdto)
 	if (ret)
 		goto err;
 
+	ret = overlay_symbol_update(fdt, fdto);
+	if (ret)
+		goto err;
+
 	/*
 	 * The overlay has been damaged, erase its magic.
 	 */