On May 27, 2016, at 12:13 , Maxime Ripard maxime.ripard@free-electrons.com wrote:

Having dashes as a separator in the DTB name is a quite common practice.

However, the current code to generate objects from DTBs assumes the separator is an underscore, leading to a compilation error when building a device tree with dashes.

Replace all the dashes in the DTB name to generate the symbols name, which should solve this issue.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

scripts/Makefile.lib | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index ad1d9b5d7dd2..df1b5a7b4150 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -275,11 +275,11 @@ cmd_dt_S_dtb= \ ( \ echo '.section .dtb.init.rodata,"a"'; \ echo '.balign 16'; \

• echo '.global __dtb_$(*F)_begin'; \
• echo '__dtb_$(*F)_begin:'; \

• echo '.global __dtb_$(subst -,_,$(*F))_begin'; \
• echo '__dtb_$(subst -,_,$(*F))_begin:'; \ echo '.incbin "$<" '; \

• echo '__dtb_$(*F)_end:'; \
• echo '.global __dtb_$(*F)_end'; \

• echo '__dtb_$(subst -,_,$(*F))_end:'; \
• echo '.global __dtb_$(subst -,_,$(*F))_end'; \ echo '.balign 16'; \

) > $@

-- 2.8.2

Acked-by: Pantelis Antoniou pantelis.antoniou@konsulko.com

Hi Maxime,

On May 27, 2016, at 12:13 , Maxime Ripard maxime.ripard@free-electrons.com wrote:

The libfdt overlay support introduces a bunch of new includes and functions.

Make sure we are able to build it by adding the needed glue.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

include/libfdt_env.h | 7 +++++++ 1 file changed, 7 insertions(+)

diff --git a/include/libfdt_env.h b/include/libfdt_env.h index 273b5d30f867..2d2196031332 100644 --- a/include/libfdt_env.h +++ b/include/libfdt_env.h @@ -23,6 +23,13 @@ typedef __be64 fdt64_t; #define fdt64_to_cpu(x) be64_to_cpu(x) #define cpu_to_fdt64(x) cpu_to_be64(x)

+#ifdef __UBOOT__ +#include "malloc.h" +#include "vsprintf.h"

+#define strtol(cp, endp, base) simple_strtol(cp, endp, base) +#endif

/* adding a ramdisk needs 0x44 bytes in version 2008.10 */ #define FDT_RAMDISK_OVERHEAD 0x80

We need to figure out what the upstream libfdt/dtc maintainer’s take is on this is. For u-boot we’re fine and for now it’s OK.

Acked-by: Pantelis Antoniou pantelis.antoniou@konsulko.com

-- 2.8.2

Hi David,

On Sat, Jun 11, 2016 at 08:30:35PM +1000, David Gibson wrote:

On Fri, Jun 10, 2016 at 05:03:36PM +0300, Pantelis Antoniou wrote:

...

Hi Maxime,

...

On May 27, 2016, at 12:13 , Maxime Ripard maxime.ripard@free-electrons.com wrote:

The libfdt overlay support introduces a bunch of new includes and functions.

Make sure we are able to build it by adding the needed glue.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

include/libfdt_env.h | 7 +++++++ 1 file changed, 7 insertions(+)

diff --git a/include/libfdt_env.h b/include/libfdt_env.h index 273b5d30f867..2d2196031332 100644 --- a/include/libfdt_env.h +++ b/include/libfdt_env.h @@ -23,6 +23,13 @@ typedef __be64 fdt64_t; #define fdt64_to_cpu(x) be64_to_cpu(x) #define cpu_to_fdt64(x) cpu_to_be64(x)

+#ifdef __UBOOT__ +#include "malloc.h" +#include "vsprintf.h"

+#define strtol(cp, endp, base) simple_strtol(cp, endp, base) +#endif

/* adding a ramdisk needs 0x44 bytes in version 2008.10 */ #define FDT_RAMDISK_OVERHEAD 0x80

We need to figure out what the upstream libfdt/dtc maintainer’s take is on this is. For u-boot we’re fine and for now it’s OK.

These were sent to the upstream dtc list as well.

The concept is fine, but there are a number of problems in the implementation. I sent detailed review comments on the upstream versions, haven't seen a respin yet.

Yes, thanks a lot for your comments, I'll address them and resend a new serie.

(it might not be before a couple of weeks though).

Thanks! Maxime

On May 27, 2016, at 12:13 , Maxime Ripard maxime.ripard@free-electrons.com wrote:

Implement a macro based on fdt_first_property_offset and fdt_next_property_offset that provides a convenience to iterate over all the properties of a given node.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

include/libfdt.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)

diff --git a/include/libfdt.h b/include/libfdt.h index 74b1d149c2dd..4e8eb9ede3a4 100644 --- a/include/libfdt.h +++ b/include/libfdt.h @@ -441,6 +441,30 @@ int fdt_first_property_offset(const void *fdt, int nodeoffset); int fdt_next_property_offset(const void *fdt, int offset);

/**

• • fdt_for_each_property - iterate over all properties of a node
• • @fdt: FDT blob (const void *)
• • @node: node offset (int)
• • @property: property offset (int)
• • This is actually a wrapper around a for loop and would be used like so:
• • fdt_for_each_property(fdt, node, property) {

• • ...
    

• • use property
    

• • ...
    

• • }
• • Note that this is implemented as a macro and property is used as
• • iterator in the loop. It should therefore be a locally allocated
• • variable. The node variable on the other hand is never modified, so
• • it can be constant or even a literal.
• */

+#define fdt_for_each_property(fdt, node, property) \

• for (property = fdt_first_property_offset(fdt, node); \

•     property >= 0;					\
  

•     property = fdt_next_property_offset(fdt, property))
  

+/**

• fdt_get_property_by_offset - retrieve the property at a given offset
• @fdt: pointer to the device tree blob
• @offset: offset of the property to retrieve

-- 2.8.2

Acked-by: Pantelis Antoniou pantelis.antoniou@konsulko.com

I’d like to see this merged in dtc upstream please too.

On May 27, 2016, at 12:13 , Maxime Ripard maxime.ripard@free-electrons.com wrote:

Add a function to retrieve the highest phandle in a given device tree.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

include/libfdt.h | 13 +++++++++++++ lib/libfdt/fdt_ro.c | 15 +++++++++++++++ 2 files changed, 28 insertions(+)

diff --git a/include/libfdt.h b/include/libfdt.h index 4e8eb9ede3a4..1e01b2c7ebdf 100644 --- a/include/libfdt.h +++ b/include/libfdt.h @@ -284,6 +284,19 @@ int fdt_move(const void *fdt, void *buf, int bufsize); const char *fdt_string(const void *fdt, int stroffset);

/**

• • fdt_get_max_phandle - retrieves the highest phandle in a tree
• • @fdt: pointer to the device tree blob
• • fdt_get_max_phandle retrieves the highest phandlle in the given
• • device tree

^ typo; phandlle -> phandle

• • returns:

• •  the highest phandle on success
    

• •  0, if an error occured
    

• */

+uint32_t fdt_get_max_phandle(const void *fdt);

+/**

• fdt_num_mem_rsv - retrieve the number of memory reserve map entries
• @fdt: pointer to the device tree blob

diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c index 7b0777b67eb3..9130ce718965 100644 --- a/lib/libfdt/fdt_ro.c +++ b/lib/libfdt/fdt_ro.c @@ -47,6 +47,21 @@ static int _fdt_string_eq(const void *fdt, int stroffset, return (strnlen(p, len + 1) == len) && (memcmp(p, s, len) == 0); }

+uint32_t fdt_get_max_phandle(const void *fdt) +{

• uint32_t max_phandle = 0, phandle;
• int offset;
• for (offset = fdt_next_node(fdt, -1, NULL); offset >= 0;

•     offset = fdt_next_node(fdt, offset, NULL)) {
  

• phandle = fdt_get_phandle(fdt, offset);
  

• if (phandle > max_phandle)
  

• 	max_phandle = phandle;
  

• }
• return max_phandle;

+}

int fdt_get_mem_rsv(const void *fdt, int n, uint64_t *address, uint64_t *size) { FDT_CHECK_HEADER(fdt); -- 2.8.2

Regards

— Pantelis

On 2016-05-27 02:13, Maxime Ripard wrote:

The device tree overlays are a good way to deal with user-modifyable boards or boards with some kind of an expansion mechanism where we can easily plug new board in (like the BBB, the Raspberry Pi or the CHIP).

Add a new function to merge overlays with a base device tree.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

include/libfdt.h | 30 ++++ lib/libfdt/Makefile | 2 +- lib/libfdt/fdt_overlay.c | 414 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 445 insertions(+), 1 deletion(-) create mode 100644 lib/libfdt/fdt_overlay.c

How big is the bloat? Did you thought about making it optional? e.g. CONFIG_OF_LIBFDT_OVERLAY

What is the general opinion on that?

diff --git a/include/libfdt.h b/include/libfdt.h index 1e01b2c7ebdf..783067e841a1 100644 --- a/include/libfdt.h +++ b/include/libfdt.h @@ -1698,6 +1698,36 @@ int fdt_add_subnode(void *fdt, int parentoffset, const char *name); */ int fdt_del_node(void *fdt, int nodeoffset);

+/**

• • fdt_overlay_apply - Applies a DT overlay on a base DT
• • @fdt: pointer to the base device tree blob
• • @fdto: pointer to the device tree overlay blob
• • fdt_overlay_apply() will apply the given device tree overlay on the
• • given base device tree.
• • Expect the base device tree to be modified, even if the function
• • returns an error.

Hm this sounds somewhat contradicting to a comment inside that function...

• • returns:
• • 0, on success
• • -FDT_ERR_NOSPACE, there's not enough space in the base device tree
• • -FDT_ERR_NOTFOUND, the overlay points to some inexistant nodes or

• • properties in the base DT
    

• • -FDT_ERR_BADPHANDLE, the phandles in the overlay do not have the right

• • magic
    

• • -FDT_ERR_INTERNAL,
• • -FDT_ERR_BADLAYOUT,
• • -FDT_ERR_BADMAGIC,
• • -FDT_ERR_BADOFFSET,
• • -FDT_ERR_BADPATH,
• • -FDT_ERR_BADVERSION,
• • -FDT_ERR_BADSTRUCTURE,
• • -FDT_ERR_BADSTATE,
• • -FDT_ERR_TRUNCATED, standard meanings
• */

+int fdt_overlay_apply(void *fdt, void *fdto);

/**********************************************************************/ /* Debugging / informational functions */ /**********************************************************************/ diff --git a/lib/libfdt/Makefile b/lib/libfdt/Makefile index 934d6142c3e9..4935bf012a75 100644 --- a/lib/libfdt/Makefile +++ b/lib/libfdt/Makefile @@ -6,4 +6,4 @@ #

obj-y += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o \

• fdt_empty_tree.o fdt_addresses.o fdt_region.o

• fdt_empty_tree.o fdt_addresses.o fdt_region.o fdt_overlay.o

diff --git a/lib/libfdt/fdt_overlay.c b/lib/libfdt/fdt_overlay.c new file mode 100644 index 000000000000..1e68e903c734 --- /dev/null +++ b/lib/libfdt/fdt_overlay.c @@ -0,0 +1,414 @@ +#include "libfdt_env.h"

+#include <fdt.h> +#include <libfdt.h>

+#include "libfdt_internal.h"

+static uint32_t fdt_overlay_get_target_phandle(const void *fdto, int node) +{

• const uint32_t *val;
• int len;
• val = fdt_getprop(fdto, node, "target", &len);
• if (!val || (len != sizeof(*val)))

• return 0;
  

• return fdt32_to_cpu(*val);

+}

+static int fdt_overlay_get_target(const void *fdt, const void *fdto,

• 		  int fragment)
  

+{

• uint32_t phandle;
• const char *path;
• /* Try first to do a phandle based lookup */
• phandle = fdt_overlay_get_target_phandle(fdto, fragment);
• if (phandle)

• return fdt_node_offset_by_phandle(fdt, phandle);
  

• /* And then a path based lookup */
• path = fdt_getprop(fdto, fragment, "target-path", NULL);
• if (!path)

• return -FDT_ERR_NOTFOUND;
  

• return fdt_path_offset(fdt, path);

+}

+static int fdt_overlay_adjust_node_phandles(void *fdto, int node,

• 			    uint32_t delta)
  

+{

• int property;
• int child;
• fdt_for_each_property(fdto, node, property) {

• const uint32_t *val;
  

• const char *name;
  

• uint32_t adj_val;
  

• int len;
  

• int ret;
  

• val = fdt_getprop_by_offset(fdto, property,
  

• 			    &name, &len);
  

• if (!val || (len != sizeof(*val)))
  

• 	continue;
  

• if (strcmp(name, "phandle") && strcmp(name, "linux,phandle"))
  

• 	continue;
  

• adj_val = fdt32_to_cpu(*val);
  

• adj_val += delta;
  

• ret = fdt_setprop_inplace_u32(fdto, node, name, adj_val);
  

• if (ret)
  

• 	return ret;
  

• }
• fdt_for_each_subnode(fdto, child, node)

• fdt_overlay_adjust_node_phandles(fdto, child, delta);
  

• return 0;

+}

+static int fdt_overlay_adjust_local_phandles(void *fdto, uint32_t delta) +{

• int root;
• root = fdt_path_offset(fdto, "/");
• if (root < 0)

• return root;
  

• return fdt_overlay_adjust_node_phandles(fdto, root, delta);

+}

+static int _fdt_overlay_update_local_references(void *fdto,

• 				int tree_node,
  

• 				int fixup_node,
  

• 				uint32_t delta)
  

+{

• int fixup_prop;
• int fixup_child;
• fdt_for_each_property(fdto, fixup_node, fixup_prop) {

• const char *fixup_name, *tree_name;
  

• const uint32_t *val = NULL;
  

• uint32_t adj_val;
  

• int fixup_len;
  

• int tree_prop;
  

• int tree_len;
  

• int ret;
  

• fdt_getprop_by_offset(fdto, fixup_prop,
  

• 		      &fixup_name, &fixup_len);
  

• if (!strcmp(fixup_name, "phandle") ||
  

•     !strcmp(fixup_name, "linux,phandle"))
  

• 	continue;
  

• fdt_for_each_property(fdto, tree_node, tree_prop) {
  

• 	val = fdt_getprop_by_offset(fdto, tree_prop,
  

• 				    &tree_name, &tree_len);
  

• 	if (!strcmp(tree_name, fixup_name))
  

• 		break;
  

• }
  

• if (!val || !tree_len)
  

• 	return -FDT_ERR_NOTFOUND;
  

• if (!tree_name)
  

• 	return -FDT_ERR_NOTFOUND;
  

• if (tree_len != 4)
  

• 	return -FDT_ERR_NOTFOUND;
  

• adj_val = fdt32_to_cpu(*val);
  

• adj_val += delta;
  

• ret = fdt_setprop_inplace_u32(fdto, tree_node, fixup_name,
  

• 			      adj_val);
  

• if (ret)
  

• 	return ret;
  

• }
• fdt_for_each_subnode(fdto, fixup_child, fixup_node) {

• const char *fixup_child_name = fdt_get_name(fdto,
  

• 					    fixup_child, NULL);
  

• int tree_child;
  

• fdt_for_each_subnode(fdto, tree_child, tree_node) {
  

• 	const char *tree_child_name = fdt_get_name(fdto,
  

• 						   tree_child,
  

• 						   NULL);
  

• 	if (!strcmp(fixup_child_name, tree_child_name))
  

• 		break;
  

• }
  

• _fdt_overlay_update_local_references(fdto,
  

• 				     tree_child,
  

• 				     fixup_child,
  

• 				     delta);
  

• }
• return 0;

+}

+static int fdt_overlay_update_local_references(void *dto, uint32_t delta) +{

• int root, fixups;
• root = fdt_path_offset(dto, "/");
• if (root < 0)

• return root;
  

• fixups = fdt_path_offset(dto, "/__local_fixups__");
• if (root < 0)

• return root;
  

• return _fdt_overlay_update_local_references(dto, root, fixups,

• 				    delta);
  

+}

+static int _fdt_overlay_fixup_phandle(void *fdt, void *fdto,

• 		      int symbols_off,
  

• 		      const char *path, const char *name,
  

• 		      int index, const char *label)
  

+{

• const uint32_t *prop_val;
• const char *symbol_path;
• uint32_t *fixup_val;
• uint32_t phandle;
• int symbol_off, fixup_off;
• int prop_len;
• int ret;
• symbol_path = fdt_getprop(fdt, symbols_off, label,

• 		  &prop_len);
  

• if (!symbol_path)

• return -FDT_ERR_NOTFOUND;
  

• symbol_off = fdt_path_offset(fdt, symbol_path);
• if (symbol_off < 0)

• return symbol_off;
  

• phandle = fdt_get_phandle(fdt, symbol_off);
• if (!phandle)

• return -FDT_ERR_NOTFOUND;
  

• fixup_off = fdt_path_offset(fdto, path);
• if (fixup_off < 0)

• return fixup_off;
  

• prop_val = fdt_getprop(fdto, fixup_off, name,

• 	       &prop_len);
  

• if (!prop_val)

• return -FDT_ERR_NOTFOUND;
  

• fixup_val = malloc(prop_len);
• if (!fixup_val)

• return -FDT_ERR_INTERNAL;
  

• memcpy(fixup_val, prop_val, prop_len);
• if (fdt32_to_cpu(fixup_val[index]) != 0xdeadbeef) {

• ret = -FDT_ERR_BADPHANDLE;
  

• goto out;
  

• }
• fixup_val[index] = cpu_to_fdt32(phandle);
• ret = fdt_setprop_inplace(fdto, fixup_off,

• 		  name, fixup_val,
  

• 		  prop_len);
  

+out:

• free(fixup_val);
• return ret;

+};

+static int fdt_overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off,

• 		     int property)
  

+{

• const char *value, *tmp_value;
• const char *label;
• int tmp_len, len, next;
• int table = 0;
• int i;
• value = fdt_getprop_by_offset(fdto, property,

• 		      &label, &len);
  

• tmp_value = value;
• tmp_len = len;
• do {

• next = strlen(tmp_value) + 1;
  

• tmp_len -= next;
  

• tmp_value += next;
  

• table++;
  

• } while (tmp_len > 0);
• for (i = 0; i < table; i++) {

• const char *prop_string = value;
  

• const char *path, *name;
  

• char *prop_cpy, *prop_tmp;
  

• int index, stlen;
  

• char *sep;
  

• stlen = strlen(prop_string);
  

• prop_cpy = malloc(stlen + 1);
  

• if (!prop_cpy)
  

• 	return -FDT_ERR_INTERNAL;
  

A malloc in a inner loop sounds somewhat scary to me, but it probably doesn't matter that much, not sure how memory management in U-Boot exactly works.

I tried to find a maximum property length in order to use a stack allocated variable, but it seems that the property length is unlimited...

What you might try is using realloc. It should only resize the allocation if the requested size is larger. Initialize with 255, and then realloc in here, and free at the end of the function....

• strncpy(prop_cpy, prop_string, stlen);
  

• prop_cpy[stlen] = '\0';
  

• for (prop_tmp = prop_cpy; (sep = strchr(prop_tmp, ':'));
  

•      prop_tmp += ((sep - prop_tmp) + 1))
  

• 	*sep = '\0';
  

• prop_tmp = prop_cpy;
  

• path = prop_tmp;
  

• prop_tmp += strlen(prop_tmp) + 1;
  

• name = prop_tmp;
  

• prop_tmp += strlen(prop_tmp) + 1;
  

• index = strtol(prop_tmp, NULL, 10);
  

• prop_tmp += strlen(prop_tmp) + 1;
  

• value += stlen + 1;
  

• len -= stlen + 1;
  

• _fdt_overlay_fixup_phandle(fdt, fdto, symbols_off,
  

• 			   path, name, index, label);
  

• free(prop_cpy);
  

• }
• return 0;

+}

+static int fdt_overlay_fixup_phandles(void *dt, void *dto) +{

• int fixups_off, symbols_off;
• int property;
• symbols_off = fdt_path_offset(dt, "/__symbols__");
• fixups_off = fdt_path_offset(dto, "/__fixups__");
• fdt_for_each_property(dto, fixups_off, property)

• fdt_overlay_fixup_phandle(dt, dto, symbols_off, property);
  

• return 0;

+}

+static int fdt_apply_overlay_node(void *dt, void *dto,

• 		  int target, int overlay)
  

+{

• int property;
• int node;
• fdt_for_each_property(dto, overlay, property) {

• const char *name;
  

• const void *prop;
  

• int prop_len;
  

• int ret;
  

• prop = fdt_getprop_by_offset(dto, property, &name,
  

• 			     &prop_len);
  

• if (!prop)
  

• 	return -FDT_ERR_INTERNAL;
  

• ret = fdt_setprop(dt, target, name, prop, prop_len);
  

• if (ret)
  

• 	return ret;
  

• }
• fdt_for_each_subnode(dto, node, overlay) {

• const char *name = fdt_get_name(dto, node, NULL);
  

• int nnode;
  

• int ret;
  

• nnode = fdt_add_subnode(dt, target, name);
  

• if (nnode < 0)
  

• 	return nnode;
  

• ret = fdt_apply_overlay_node(dt, dto, nnode, node);
  

• if (ret)
  

• 	return ret;
  

• }
• return 0;

+}

+static int fdt_overlay_merge(void *dt, void *dto) +{

• int root, fragment;
• root = fdt_path_offset(dto, "/");
• if (root < 0)

• return root;
  

• fdt_for_each_subnode(dto, fragment, root) {

• const char *name = fdt_get_name(dto, fragment, NULL);
  

• uint32_t target;
  

• int overlay;
  

• int ret;
  

• if (strncmp(name, "fragment", 8))
  

• 	continue;
  

• target = fdt_overlay_get_target(dt, dto, fragment);
  

• if (target < 0)
  

• 	return target;
  

• overlay = fdt_subnode_offset(dto, fragment, "__overlay__");
  

• if (overlay < 0)
  

• 	return overlay;
  

• ret = fdt_apply_overlay_node(dt, dto, target, overlay);
  

• if (ret)
  

• 	return ret;
  

• }
• return 0;

+}

+int fdt_overlay_apply(void *fdt, void *fdto) +{

• uint32_t delta = fdt_get_max_phandle(fdt) + 1;
• void *fdto_copy;
• int ret;
• FDT_CHECK_HEADER(fdt);
• FDT_CHECK_HEADER(fdto);
• /*

• * We're going to modify the overlay so that we can apply it.
  

• *
  

• * Make sure sure we don't destroy the original
  

• */
  

... here.

• fdto_copy = malloc(fdt_totalsize(fdto));
• if (!fdto_copy)

• return -FDT_ERR_INTERNAL;
  

• ret = fdt_move(fdto, fdto_copy, fdt_totalsize(fdto));
• if (ret)

• goto out;
  

• ret = fdt_overlay_adjust_local_phandles(fdto_copy, delta);
• if (ret)

• goto out;
  

• ret = fdt_overlay_update_local_references(fdto_copy, delta);
• if (ret)

• goto out;
  

• ret = fdt_overlay_fixup_phandles(fdt, fdto_copy);
• if (ret)

• goto out;
  

• ret = fdt_overlay_merge(fdt, fdto_copy);

+out:

• free(fdto_copy);
• return ret;

+}

-- Stefan

Hi Pantelis,

On Fri, Jun 10, 2016 at 05:28:11PM +0300, Pantelis Antoniou wrote:

Hi Maxime,

...

On May 27, 2016, at 12:13 , Maxime Ripard maxime.ripard@free-electrons.com wrote:

The device tree overlays are a good way to deal with user-modifyable boards or boards with some kind of an expansion mechanism where we can easily plug new board in (like the BBB, the Raspberry Pi or the CHIP).

Add a new function to merge overlays with a base device tree.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

include/libfdt.h | 30 ++++ lib/libfdt/Makefile | 2 +- lib/libfdt/fdt_overlay.c | 414 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 445 insertions(+), 1 deletion(-) create mode 100644 lib/libfdt/fdt_overlay.c

diff --git a/include/libfdt.h b/include/libfdt.h index 1e01b2c7ebdf..783067e841a1 100644 --- a/include/libfdt.h +++ b/include/libfdt.h @@ -1698,6 +1698,36 @@ int fdt_add_subnode(void *fdt, int parentoffset, const char *name); */ int fdt_del_node(void *fdt, int nodeoffset);

+/**

• • fdt_overlay_apply - Applies a DT overlay on a base DT
• • @fdt: pointer to the base device tree blob
• • @fdto: pointer to the device tree overlay blob
• • fdt_overlay_apply() will apply the given device tree overlay on the
• • given base device tree.
• • Expect the base device tree to be modified, even if the function
• • returns an error.

If the base tree has been modified on an error it is unsafe to use it for booting. A valid strategy would be to scribble over the DT magic number so that that blob is invalidated.

What are the other people’s opinion on this?

That would probably be safer yes, I'll change that.

...

+static int fdt_overlay_get_target(const void *fdt, const void *fdto,

• 		  int fragment)
  

+{

• uint32_t phandle;
• const char *path;
• /* Try first to do a phandle based lookup */
• phandle = fdt_overlay_get_target_phandle(fdto, fragment);
• if (phandle)

• return fdt_node_offset_by_phandle(fdt, phandle);
  

• /* And then a path based lookup */
• path = fdt_getprop(fdto, fragment, "target-path", NULL);
• if (!path)

• return -FDT_ERR_NOTFOUND;
  

• return fdt_path_offset(fdt, path);

+}

Those targets are fine; beware there are patches with more target options.

Ack.

...

+static int fdt_overlay_merge(void *dt, void *dto) +{

• int root, fragment;
• root = fdt_path_offset(dto, "/");
• if (root < 0)

• return root;
  

• fdt_for_each_subnode(dto, fragment, root) {

• const char *name = fdt_get_name(dto, fragment, NULL);
  

• uint32_t target;
  

• int overlay;
  

• int ret;
  

• if (strncmp(name, "fragment", 8))
  

• 	continue;
  

This is incorrect. The use of “fragment” is a convention only. The real test whether the node is an overlay fragment is that it contains a target property.

...

• target = fdt_overlay_get_target(dt, dto, fragment);
  

• if (target < 0)
  

• 	return target;
  

So you could do ‘if (target < 0) continue;’ or handle a more complex error code.

Ok, will change.

I would caution against the liberal use of malloc in libfdt. We’re possibly running in a constrained environment; a custom extents based (non freeing) allocator should be better.

David had the same comments, and I will drop the mallocs entirely.

We need some figures about memory consumption when this is enabled, and a CONFIG option to disable it.

The current code (before and after that patch) adds 18kB to a sandbox_defconfig.

Maxime

Hi David,

On Jun 14, 2016, at 03:25 , David Gibson david@gibson.dropbear.id.au wrote:

On Fri, Jun 10, 2016 at 05:28:11PM +0300, Pantelis Antoniou wrote:

...

Hi Maxime,

...

On May 27, 2016, at 12:13 , Maxime Ripard maxime.ripard@free-electrons.com wrote:

The device tree overlays are a good way to deal with user-modifyable boards or boards with some kind of an expansion mechanism where we can easily plug new board in (like the BBB, the Raspberry Pi or the CHIP).

Add a new function to merge overlays with a base device tree.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

include/libfdt.h | 30 ++++ lib/libfdt/Makefile | 2 +- lib/libfdt/fdt_overlay.c | 414 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 445 insertions(+), 1 deletion(-) create mode 100644 lib/libfdt/fdt_overlay.c

diff --git a/include/libfdt.h b/include/libfdt.h index 1e01b2c7ebdf..783067e841a1 100644 --- a/include/libfdt.h +++ b/include/libfdt.h @@ -1698,6 +1698,36 @@ int fdt_add_subnode(void *fdt, int parentoffset, const char *name); */ int fdt_del_node(void *fdt, int nodeoffset);

+/**

• • fdt_overlay_apply - Applies a DT overlay on a base DT
• • @fdt: pointer to the base device tree blob
• • @fdto: pointer to the device tree overlay blob
• • fdt_overlay_apply() will apply the given device tree overlay on the
• • given base device tree.
• • Expect the base device tree to be modified, even if the function
• • returns an error.

If the base tree has been modified on an error it is unsafe to use it for booting. A valid strategy would be to scribble over the DT magic number so that that blob is invalidated.

What are the other people’s opinion on this?

...

• • returns:
• • 0, on success
• • -FDT_ERR_NOSPACE, there's not enough space in the base device tree
• • -FDT_ERR_NOTFOUND, the overlay points to some inexistant nodes or

• • properties in the base DT
    

• • -FDT_ERR_BADPHANDLE, the phandles in the overlay do not have the right

• • magic
    

• • -FDT_ERR_INTERNAL,
• • -FDT_ERR_BADLAYOUT,
• • -FDT_ERR_BADMAGIC,
• • -FDT_ERR_BADOFFSET,
• • -FDT_ERR_BADPATH,
• • -FDT_ERR_BADVERSION,
• • -FDT_ERR_BADSTRUCTURE,
• • -FDT_ERR_BADSTATE,
• • -FDT_ERR_TRUNCATED, standard meanings
• */

+int fdt_overlay_apply(void *fdt, void *fdto);

/**********************************************************************/ /* Debugging / informational functions */ /**********************************************************************/ diff --git a/lib/libfdt/Makefile b/lib/libfdt/Makefile index 934d6142c3e9..4935bf012a75 100644 --- a/lib/libfdt/Makefile +++ b/lib/libfdt/Makefile @@ -6,4 +6,4 @@ #

obj-y += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o \

• fdt_empty_tree.o fdt_addresses.o fdt_region.o

• fdt_empty_tree.o fdt_addresses.o fdt_region.o fdt_overlay.o

diff --git a/lib/libfdt/fdt_overlay.c b/lib/libfdt/fdt_overlay.c new file mode 100644 index 000000000000..1e68e903c734 --- /dev/null +++ b/lib/libfdt/fdt_overlay.c @@ -0,0 +1,414 @@ +#include "libfdt_env.h"

+#include <fdt.h> +#include <libfdt.h>

+#include "libfdt_internal.h"

+static uint32_t fdt_overlay_get_target_phandle(const void *fdto, int node) +{

• const uint32_t *val;
• int len;
• val = fdt_getprop(fdto, node, "target", &len);
• if (!val || (len != sizeof(*val)))

• return 0;
  

• return fdt32_to_cpu(*val);

+}

+static int fdt_overlay_get_target(const void *fdt, const void *fdto,

• 		  int fragment)
  

+{

• uint32_t phandle;
• const char *path;
• /* Try first to do a phandle based lookup */
• phandle = fdt_overlay_get_target_phandle(fdto, fragment);
• if (phandle)

• return fdt_node_offset_by_phandle(fdt, phandle);
  

• /* And then a path based lookup */
• path = fdt_getprop(fdto, fragment, "target-path", NULL);
• if (!path)

• return -FDT_ERR_NOTFOUND;
  

• return fdt_path_offset(fdt, path);

+}

Those targets are fine; beware there are patches with more target options.

...

+static int fdt_overlay_adjust_node_phandles(void *fdto, int node,

• 			    uint32_t delta)
  

+{

• int property;
• int child;
• fdt_for_each_property(fdto, node, property) {

• const uint32_t *val;
  

• const char *name;
  

• uint32_t adj_val;
  

• int len;
  

• int ret;
  

• val = fdt_getprop_by_offset(fdto, property,
  

• 			    &name, &len);
  

• if (!val || (len != sizeof(*val)))
  

Superfluous parentheses.

...

• 	continue;
  

• if (strcmp(name, "phandle") && strcmp(name, "linux,phandle"))
  

• 	continue;
  

• adj_val = fdt32_to_cpu(*val);
  

• adj_val += delta;
  

• ret = fdt_setprop_inplace_u32(fdto, node, name, adj_val);
  

• if (ret)
  

• 	return ret;
  

• }
• fdt_for_each_subnode(fdto, child, node)

• fdt_overlay_adjust_node_phandles(fdto, child, delta);
  

• return 0;

+}

+static int fdt_overlay_adjust_local_phandles(void *fdto, uint32_t delta) +{

• int root;
• root = fdt_path_offset(fdto, "/");
• if (root < 0)

• return root;
  

• return fdt_overlay_adjust_node_phandles(fdto, root, delta);

+}

+static int _fdt_overlay_update_local_references(void *fdto,

• 				int tree_node,
  

• 				int fixup_node,
  

• 				uint32_t delta)
  

+{

• int fixup_prop;
• int fixup_child;
• fdt_for_each_property(fdto, fixup_node, fixup_prop) {

• const char *fixup_name, *tree_name;
  

• const uint32_t *val = NULL;
  

• uint32_t adj_val;
  

• int fixup_len;
  

• int tree_prop;
  

• int tree_len;
  

• int ret;
  

• fdt_getprop_by_offset(fdto, fixup_prop,
  

• 		      &fixup_name, &fixup_len);
  

• if (!strcmp(fixup_name, "phandle") ||
  

•     !strcmp(fixup_name, "linux,phandle"))
  

• 	continue;
  

• fdt_for_each_property(fdto, tree_node, tree_prop) {
  

• 	val = fdt_getprop_by_offset(fdto, tree_prop,
  

• 				    &tree_name, &tree_len);
  

• 	if (!strcmp(tree_name, fixup_name))
  

• 		break;
  

• }
  

• if (!val || !tree_len)
  

• 	return -FDT_ERR_NOTFOUND;
  

• if (!tree_name)
  

• 	return -FDT_ERR_NOTFOUND;
  

• if (tree_len != 4)
  

• 	return -FDT_ERR_NOTFOUND;
  

• adj_val = fdt32_to_cpu(*val);
  

• adj_val += delta;
  

• ret = fdt_setprop_inplace_u32(fdto, tree_node, fixup_name,
  

• 			      adj_val);
  

• if (ret)
  

• 	return ret;
  

• }
• fdt_for_each_subnode(fdto, fixup_child, fixup_node) {

• const char *fixup_child_name = fdt_get_name(fdto,
  

• 					    fixup_child, NULL);
  

• int tree_child;
  

• fdt_for_each_subnode(fdto, tree_child, tree_node) {
  

• 	const char *tree_child_name = fdt_get_name(fdto,
  

• 						   tree_child,
  

• 						   NULL);
  

• 	if (!strcmp(fixup_child_name, tree_child_name))
  

• 		break;
  

• }
  

• _fdt_overlay_update_local_references(fdto,
  

• 				     tree_child,
  

• 				     fixup_child,
  

• 				     delta);
  

• }
• return 0;

+}

+static int fdt_overlay_update_local_references(void *dto, uint32_t delta) +{

• int root, fixups;
• root = fdt_path_offset(dto, "/");
• if (root < 0)

• return root;
  

• fixups = fdt_path_offset(dto, "/__local_fixups__");
• if (root < 0)

• return root;
  

• return _fdt_overlay_update_local_references(dto, root, fixups,

• 				    delta);
  

+}

+static int _fdt_overlay_fixup_phandle(void *fdt, void *fdto,

• 		      int symbols_off,
  

• 		      const char *path, const char *name,
  

• 		      int index, const char *label)
  

+{

• const uint32_t *prop_val;
• const char *symbol_path;
• uint32_t *fixup_val;
• uint32_t phandle;
• int symbol_off, fixup_off;
• int prop_len;
• int ret;
• symbol_path = fdt_getprop(fdt, symbols_off, label,

• 		  &prop_len);
  

• if (!symbol_path)

• return -FDT_ERR_NOTFOUND;
  

• symbol_off = fdt_path_offset(fdt, symbol_path);
• if (symbol_off < 0)

• return symbol_off;
  

• phandle = fdt_get_phandle(fdt, symbol_off);
• if (!phandle)

• return -FDT_ERR_NOTFOUND;
  

• fixup_off = fdt_path_offset(fdto, path);
• if (fixup_off < 0)

• return fixup_off;
  

• prop_val = fdt_getprop(fdto, fixup_off, name,

• 	       &prop_len);
  

• if (!prop_val)

• return -FDT_ERR_NOTFOUND;
  

• fixup_val = malloc(prop_len);
• if (!fixup_val)

• return -FDT_ERR_INTERNAL;
  

• memcpy(fixup_val, prop_val, prop_len);
• if (fdt32_to_cpu(fixup_val[index]) != 0xdeadbeef) {

• ret = -FDT_ERR_BADPHANDLE;
  

• goto out;
  

• }
• fixup_val[index] = cpu_to_fdt32(phandle);
• ret = fdt_setprop_inplace(fdto, fixup_off,

• 		  name, fixup_val,
  

• 		  prop_len);
  

+out:

• free(fixup_val);
• return ret;

+};

+static int fdt_overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off,

• 		     int property)
  

+{

• const char *value, *tmp_value;
• const char *label;
• int tmp_len, len, next;
• int table = 0;
• int i;
• value = fdt_getprop_by_offset(fdto, property,

• 		      &label, &len);
  

• tmp_value = value;
• tmp_len = len;
• do {

• next = strlen(tmp_value) + 1;
  

• tmp_len -= next;
  

• tmp_value += next;
  

• table++;
  

• } while (tmp_len > 0);
• for (i = 0; i < table; i++) {

• const char *prop_string = value;
  

• const char *path, *name;
  

• char *prop_cpy, *prop_tmp;
  

• int index, stlen;
  

• char *sep;
  

• stlen = strlen(prop_string);
  

• prop_cpy = malloc(stlen + 1);
  

• if (!prop_cpy)
  

• 	return -FDT_ERR_INTERNAL;
  

• strncpy(prop_cpy, prop_string, stlen);
  

• prop_cpy[stlen] = '\0';
  

• for (prop_tmp = prop_cpy; (sep = strchr(prop_tmp, ':'));
  

•      prop_tmp += ((sep - prop_tmp) + 1))
  

• 	*sep = '\0';
  

• prop_tmp = prop_cpy;
  

• path = prop_tmp;
  

• prop_tmp += strlen(prop_tmp) + 1;
  

• name = prop_tmp;
  

• prop_tmp += strlen(prop_tmp) + 1;
  

• index = strtol(prop_tmp, NULL, 10);
  

• prop_tmp += strlen(prop_tmp) + 1;
  

• value += stlen + 1;
  

• len -= stlen + 1;
  

• _fdt_overlay_fixup_phandle(fdt, fdto, symbols_off,
  

• 			   path, name, index, label);
  

• free(prop_cpy);
  

• }
• return 0;

+}

+static int fdt_overlay_fixup_phandles(void *dt, void *dto) +{

• int fixups_off, symbols_off;
• int property;
• symbols_off = fdt_path_offset(dt, "/__symbols__");
• fixups_off = fdt_path_offset(dto, "/__fixups__");
• fdt_for_each_property(dto, fixups_off, property)

• fdt_overlay_fixup_phandle(dt, dto, symbols_off, property);
  

• return 0;

+}

+static int fdt_apply_overlay_node(void *dt, void *dto,

• 		  int target, int overlay)
  

+{

• int property;
• int node;
• fdt_for_each_property(dto, overlay, property) {

• const char *name;
  

• const void *prop;
  

• int prop_len;
  

• int ret;
  

• prop = fdt_getprop_by_offset(dto, property, &name,
  

• 			     &prop_len);
  

• if (!prop)
  

• 	return -FDT_ERR_INTERNAL;
  

• ret = fdt_setprop(dt, target, name, prop, prop_len);
  

• if (ret)
  

• 	return ret;
  

• }
• fdt_for_each_subnode(dto, node, overlay) {

• const char *name = fdt_get_name(dto, node, NULL);
  

• int nnode;
  

• int ret;
  

• nnode = fdt_add_subnode(dt, target, name);
  

• if (nnode < 0)
  

• 	return nnode;
  

• ret = fdt_apply_overlay_node(dt, dto, nnode, node);
  

• if (ret)
  

• 	return ret;
  

• }
• return 0;

+}

+static int fdt_overlay_merge(void *dt, void *dto) +{

• int root, fragment;
• root = fdt_path_offset(dto, "/");
• if (root < 0)

• return root;
  

• fdt_for_each_subnode(dto, fragment, root) {

• const char *name = fdt_get_name(dto, fragment, NULL);
  

• uint32_t target;
  

• int overlay;
  

• int ret;
  

• if (strncmp(name, "fragment", 8))
  

• 	continue;
  

This is incorrect. The use of “fragment” is a convention only. The real test whether the node is an overlay fragment is that it contains a target property.

Hmm.. I dislike that approach. First, it means that if new target types are introduced in future, older code is likely to silently ignore such fragments instead of realizing that it doesn't know how to apply themm. Second, it raises weird issues if some node down within a fragment also happens to have a property called "target”.

Not really. If new targets are introduced then the fragment will be ignored.

We can have an argument about what is better to do (report an error or ignore a fragment) but what it comes down to is that that applicator does not know how to handle the new target method.

There is no issues with any target properties inside a fragment because the check is not made recursively.

...

...

• target = fdt_overlay_get_target(dt, dto, fragment);
  

• if (target < 0)
  

• 	return target;
  

So you could do ‘if (target < 0) continue;’ or handle a more complex error code.

...

• overlay = fdt_subnode_offset(dto, fragment, "__overlay__");
  

• if (overlay < 0)
  

• 	return overlay;
  

• ret = fdt_apply_overlay_node(dt, dto, target, overlay);
  

• if (ret)
  

• 	return ret;
  

• }
• return 0;

+}

+int fdt_overlay_apply(void *fdt, void *fdto) +{

• uint32_t delta = fdt_get_max_phandle(fdt) + 1;
• void *fdto_copy;
• int ret;
• FDT_CHECK_HEADER(fdt);
• FDT_CHECK_HEADER(fdto);
• /*

• * We're going to modify the overlay so that we can apply it.
  

• *
  

• * Make sure sure we don't destroy the original
  

• */
  

• fdto_copy = malloc(fdt_totalsize(fdto));
• if (!fdto_copy)

• return -FDT_ERR_INTERNAL;
  

• ret = fdt_move(fdto, fdto_copy, fdt_totalsize(fdto));
• if (ret)

• goto out;
  

• ret = fdt_overlay_adjust_local_phandles(fdto_copy, delta);
• if (ret)

• goto out;
  

• ret = fdt_overlay_update_local_references(fdto_copy, delta);
• if (ret)

• goto out;
  

• ret = fdt_overlay_fixup_phandles(fdt, fdto_copy);
• if (ret)

• goto out;
  

• ret = fdt_overlay_merge(fdt, fdto_copy);

+out:

• free(fdto_copy);
• return ret;

+}

I would caution against the liberal use of malloc in libfdt. We’re possibly running in a constrained environment; a custom extents based (non freeing) allocator should be better.

We need some figures about memory consumption when this is enabled, and a CONFIG option to disable it.

Regards

— Pantelis

Regards

— Pantelis

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

On 2016-05-27 02:13, Maxime Ripard wrote:

The device tree overlays are a good way to deal with user-modifyable boards or boards with some kind of an expansion mechanism where we can easily plug new board in (like the BBB or the raspberry pi).

However, so far, the usual mechanism to deal with it was to have in Linux some driver detecting the expansion boards plugged in and then request these overlays using the firmware interface.

That works in most cases, but in some cases, you might want to have the overlays applied before the userspace comes in. Either because the new board requires some kind of an early initialization, or because your root filesystem is accessed through that expansion board.

The easiest solution in such a case is to simply have the component before Linux applying that overlay, removing all these drawbacks.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

cmd/fdt.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)

Reviewed-by: Stefan Agner stefan@agner.ch

diff --git a/cmd/fdt.c b/cmd/fdt.c index 0f5923e75a41..6cbc9e525d6c 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -639,6 +639,25 @@ static int do_fdt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) #endif

}

• /* apply an overlay */
• else if (strncmp(argv[1], "ap", 2) == 0) {

• unsigned long addr;
  

• struct fdt_header *blob;
  

• if (argc != 3)
  

• 	return CMD_RET_USAGE;
  

• if (!working_fdt)
  

• 	return CMD_RET_FAILURE;
  

• addr = simple_strtoul(argv[2], NULL, 16);
  

• blob = map_sysmem(addr, 0);
  

• if (!fdt_valid(&blob))
  

• 	return CMD_RET_FAILURE;
  

• if (fdt_overlay_apply(working_fdt, blob))
  

• 	return CMD_RET_FAILURE;
  

• } /* resize the fdt */ else if (strncmp(argv[1], "re", 2) == 0) { fdt_shrink_to_minimum(working_fdt);

@@ -1025,6 +1044,7 @@ static int fdt_print(const char *pathp, char *prop, int depth) #ifdef CONFIG_SYS_LONGHELP static char fdt_help_text[] = "addr [-c] <addr> [<length>] - Set the [control] fdt location to <addr>\n"

• "fdt apply <addr> - Apply overlay to the DT\n"

#ifdef CONFIG_OF_BOARD_SETUP "fdt boardsetup - Do board-specific set up\n" #endif

On May 27, 2016, at 12:13 , Maxime Ripard maxime.ripard@free-electrons.com wrote:

This adds a bunch of unit tests for the "fdt apply" command.

They've all been run successfully in the sandbox. However, as you still require an out-of-tree dtc with overlay support, this is disabled by default.

Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com

Makefile | 1 + include/test/overlay.h | 16 ++++ include/test/suites.h | 1 + test/Kconfig | 1 + test/cmd_ut.c | 6 ++ test/overlay/Kconfig | 10 +++ test/overlay/Makefile | 15 ++++ test/overlay/cmd_ut_overlay.c | 176 ++++++++++++++++++++++++++++++++++++++ test/overlay/test-fdt-base.dts | 17 ++++ test/overlay/test-fdt-overlay.dts | 60 +++++++++++++ 10 files changed, 303 insertions(+) create mode 100644 include/test/overlay.h create mode 100644 test/overlay/Kconfig create mode 100644 test/overlay/Makefile create mode 100644 test/overlay/cmd_ut_overlay.c create mode 100644 test/overlay/test-fdt-base.dts create mode 100644 test/overlay/test-fdt-overlay.dts

diff --git a/Makefile b/Makefile index 954a865381af..71272501a75e 100644 --- a/Makefile +++ b/Makefile @@ -665,6 +665,7 @@ libs-$(CONFIG_HAS_POST) += post/ libs-y += test/ libs-y += test/dm/ libs-$(CONFIG_UT_ENV) += test/env/ +libs-$(CONFIG_UT_OVERLAY) += test/overlay/

libs-y += $(if $(BOARDDIR),board/$(BOARDDIR)/)

diff --git a/include/test/overlay.h b/include/test/overlay.h new file mode 100644 index 000000000000..392f28ff8405 --- /dev/null +++ b/include/test/overlay.h @@ -0,0 +1,16 @@ +/*

• • Copyright (c) 2016 NextThing Co
• • Copyright (c) 2016 Free Electrons
• • SPDX-License-Identifier: GPL-2.0+
• */

+#ifndef __TEST_OVERLAY_H__ +#define __TEST_OVERLAY_H__

+#include <test/test.h>

+/* Declare a new environment test */ +#define OVERLAY_TEST(_name, _flags) UNIT_TEST(_name, _flags, overlay_test)

+#endif /* __TEST_OVERLAY_H__ */ diff --git a/include/test/suites.h b/include/test/suites.h index f5790333ff8e..0e94feb07a79 100644 --- a/include/test/suites.h +++ b/include/test/suites.h @@ -10,6 +10,7 @@

int do_ut_dm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); int do_ut_env(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); +int do_ut_overlay(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); int do_ut_time(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);

#endif /* __TEST_SUITES_H__ */ diff --git a/test/Kconfig b/test/Kconfig index d71c332eee27..3643761bc6ef 100644 --- a/test/Kconfig +++ b/test/Kconfig @@ -17,3 +17,4 @@ config UT_TIME

source "test/dm/Kconfig" source "test/env/Kconfig" +source "test/overlay/Kconfig" diff --git a/test/cmd_ut.c b/test/cmd_ut.c index f6e1f413db7f..14333423a178 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -19,6 +19,9 @@ static cmd_tbl_t cmd_ut_sub[] = { #if defined(CONFIG_UT_ENV) U_BOOT_CMD_MKENT(env, CONFIG_SYS_MAXARGS, 1, do_ut_env, "", ""), #endif +#ifdef CONFIG_UT_OVERLAY

• U_BOOT_CMD_MKENT(overlay, CONFIG_SYS_MAXARGS, 1, do_ut_overlay, "", ""),

+#endif #ifdef CONFIG_UT_TIME U_BOOT_CMD_MKENT(time, CONFIG_SYS_MAXARGS, 1, do_ut_time, "", ""), #endif @@ -68,6 +71,9 @@ static char ut_help_text[] = #ifdef CONFIG_UT_ENV "ut env [test-name]\n" #endif +#ifdef CONFIG_UT_OVERLAY

• "ut overlay [test-name]\n"

+#endif #ifdef CONFIG_UT_TIME "ut time - Very basic test of time functions\n" #endif diff --git a/test/overlay/Kconfig b/test/overlay/Kconfig new file mode 100644 index 000000000000..b8e01a934373 --- /dev/null +++ b/test/overlay/Kconfig @@ -0,0 +1,10 @@ +config UT_OVERLAY

• bool "Enable Device Tree Overlays Unit Tests"
• depends on UNIT_TEST
• help

•  This enables the 'ut overlay' command which runs a series of unit
  

•  tests on the fdt overlay code.
  

•  If all is well then all tests pass although there will be a few
  

•  messages printed along the way.
  

•  Be warned that it requires an out-of-tree dtc compiler with patches
  

•  to support the DT overlays, otherwise it will fail.
  

diff --git a/test/overlay/Makefile b/test/overlay/Makefile new file mode 100644 index 000000000000..907f08544619 --- /dev/null +++ b/test/overlay/Makefile @@ -0,0 +1,15 @@ +# +# Copyright (c) 2016 NextThing Co +# Copyright (c) 2016 Free Electrons +# +# SPDX-License-Identifier: GPL-2.0+ +#

+# Test files +obj-y += cmd_ut_overlay.o

+DTC_FLAGS += -@

+# DT overlays +obj-y += test-fdt-base.dtb.o +obj-y += test-fdt-overlay.dtb.o diff --git a/test/overlay/cmd_ut_overlay.c b/test/overlay/cmd_ut_overlay.c new file mode 100644 index 000000000000..3ff1d61b7e31 --- /dev/null +++ b/test/overlay/cmd_ut_overlay.c @@ -0,0 +1,176 @@ +/*

• • Copyright (c) 2016 NextThing Co
• • Copyright (c) 2016 Free Electrons
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <command.h> +#include <errno.h> +#include <malloc.h>

+#include <linux/sizes.h>

+#include <test/ut.h> +#include <test/overlay.h>

+/* 4k ought to be enough for anybody */ +#define FDT_COPY_SIZE (4 * SZ_1K)

+extern u32 __dtb_test_fdt_base_begin; +extern u32 __dtb_test_fdt_overlay_begin;

+static int fdt_getprop_u32(void *fdt, const char *path, const char *name,

• 	   u32 *out)
  

+{

• const fdt32_t *val;
• int node_off;
• int len;
• node_off = fdt_path_offset(fdt, path);
• if (node_off < 0)

• return node_off;
  

• val = fdt_getprop(fdt, node_off, name, &len);
• if (!val || (len != sizeof(uint32_t)))

• return -FDT_ERR_NOTFOUND;
  

• *out = fdt32_to_cpu(*val);
• return 0;

+}

+static int fdt_getprop_str(void *fdt, const char *path, const char *name,

• 	   const char **out)
  

+{

• int node_off;
• node_off = fdt_path_offset(fdt, path);
• if (node_off < 0)

• return node_off;
  

• return fdt_get_string(fdt, node_off, name, out);

+}

+static int fdt_overlay_change_int_property(struct unit_test_state *uts) +{

• void *fdt = uts->priv;
• u32 val = 0;
• ut_assertok(fdt_getprop_u32(fdt, "/test-node", "test-int-property",

• 		    &val));
  

• ut_asserteq(43, val);
• return CMD_RET_SUCCESS;

+} +OVERLAY_TEST(fdt_overlay_change_int_property, 0);

+static int fdt_overlay_change_str_property(struct unit_test_state *uts) +{

• void *fdt = uts->priv;
• const char *val = NULL;
• ut_assertok(fdt_getprop_str(fdt, "/test-node", "test-str-property",

• 		    &val));
  

• ut_asserteq_str("foobar", val);
• return CMD_RET_SUCCESS;

+} +OVERLAY_TEST(fdt_overlay_change_str_property, 0);

+static int fdt_overlay_add_str_property(struct unit_test_state *uts) +{

• void *fdt = uts->priv;
• const char *val = NULL;
• ut_assertok(fdt_getprop_str(fdt, "/test-node", "test-str-property-2",

• 		    &val));
  

• ut_asserteq_str("foobar2", val);
• return CMD_RET_SUCCESS;

+} +OVERLAY_TEST(fdt_overlay_add_str_property, 0);

+static int fdt_overlay_add_node_by_phandle(struct unit_test_state *uts) +{

• void *fdt = uts->priv;
• int off;
• off = fdt_path_offset(fdt, "/test-node/new-node");
• ut_assert(off >= 0);
• ut_assertnonnull(fdt_getprop(fdt, off, "new-property", NULL));
• return CMD_RET_SUCCESS;

+} +OVERLAY_TEST(fdt_overlay_add_node_by_phandle, 0);

+static int fdt_overlay_add_node_by_path(struct unit_test_state *uts) +{

• void *fdt = uts->priv;
• int off;
• off = fdt_path_offset(fdt, "/new-node");
• ut_assert(off >= 0);
• ut_assertnonnull(fdt_getprop(fdt, off, "new-property", NULL));
• return CMD_RET_SUCCESS;

+} +OVERLAY_TEST(fdt_overlay_add_node_by_path, 0);

+int do_ut_overlay(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{

• struct unit_test *tests = ll_entry_start(struct unit_test,

• 				 overlay_test);
  

• const int n_ents = ll_entry_count(struct unit_test, overlay_test);
• struct unit_test_state *uts;
• struct unit_test *test;
• void *fdt_base = &__dtb_test_fdt_base_begin;
• void *fdt_overlay = &__dtb_test_fdt_overlay_begin;
• void *fdt_base_copy;
• ut_assertok(fdt_check_header(fdt_base));
• ut_assertok(fdt_check_header(fdt_overlay));
• uts = calloc(1, sizeof(*uts));
• if (!uts)

• return -ENOMEM;
  

• fdt_base_copy = malloc(FDT_COPY_SIZE);
• if (!fdt_base_copy)

• return -ENOMEM;
  

• uts->priv = fdt_base_copy;
• /*

• * Resize the FDT to 4k so that we have room to operate on
  

• *
  

• * (and relocate it since the memory might be mapped
  

• * read-only)
  

• */
  

• ut_assertok(fdt_open_into(fdt_base, fdt_base_copy, FDT_COPY_SIZE));
• /* Apply the overlay */
• ut_assertok(fdt_overlay_apply(fdt_base_copy, fdt_overlay));
• if (argc == 1)

• printf("Running %d environment tests\n", n_ents);
  

• for (test = tests; test < tests + n_ents; test++) {

• if (argc > 1 && strcmp(argv[1], test->name))
  

• 	continue;
  

• printf("Test: %s\n", test->name);
  

• uts->start = mallinfo();
  

• test->func(uts);
  

• }
• printf("Failures: %d\n", uts->fail_count);
• free(fdt_base_copy);
• free(uts);
• return uts->fail_count ? CMD_RET_FAILURE : 0;

+} diff --git a/test/overlay/test-fdt-base.dts b/test/overlay/test-fdt-base.dts new file mode 100644 index 000000000000..e8fa9bb5be82 --- /dev/null +++ b/test/overlay/test-fdt-base.dts @@ -0,0 +1,17 @@ +/*

• • Copyright (c) 2016 NextThing Co
• • Copyright (c) 2016 Free Electrons
• • SPDX-License-Identifier: GPL-2.0+
• */

+/dts-v1/;

+/ {

• test: test-node {

• test-int-property = <42>;
  

• test-str-property = "foo";
  

• };

+};

diff --git a/test/overlay/test-fdt-overlay.dts b/test/overlay/test-fdt-overlay.dts new file mode 100644 index 000000000000..ca3abcff287c --- /dev/null +++ b/test/overlay/test-fdt-overlay.dts @@ -0,0 +1,60 @@ +/*

• • Copyright (c) 2016 NextThing Co
• • Copyright (c) 2016 Free Electrons
• • SPDX-License-Identifier: GPL-2.0+
• */

+/dts-v1/; +/plugin/;

+/ {

• /* Test that we can change an int by another */
• fragment@0 {

• target = <&test>;
  

• __overlay__ {
  

• 	test-int-property = <43>;
  

• };
  

• };
• /* Test that we can replace a string by a longer one */
• fragment@1 {

• target = <&test>;
  

• __overlay__ {
  

• 	test-str-property = "foobar";
  

• };
  

• };
• /* Test that we add a new property */
• fragment@2 {

• target = <&test>;
  

• __overlay__ {
  

• 	test-str-property-2 = "foobar2";
  

• };
  

• };
• /* Test that we add a new node (by phandle) */
• fragment@3 {

• target = <&test>;
  

• __overlay__ {
  

• 	new-node {
  

• 		new-property;
  

• 	};
  

• };
  

• };
• /* Test that we add a new node (by path) */
• fragment@4 {

• target-path = "/";
  

• __overlay__ {
  

• 	new-node {
  

• 		new-property;
  

• 	};
  

• };
  

• };

+};

2.8.2

Acked-by: Pantelis Antoniou pantelis.antoniou@konsulko.com