Skip to contentSkip to conversation

libfdt: Add support for using aliases in fdt_path_offset()

PATCH Applied Kumar Gala 6 participants 7 messages Aug 14, 2008 – Aug 16, 2008 3 messages not in the archive

lore
Share & tools
lore

Kumar Gala

author
#0 lore raw

PATCH v3 1/1 libfdt: Add support for using aliases in fdt_path_offset()

If the path doesn't start with '/' check to see if it matches some alias under "/aliases" and substitute the matching alias value in the path and retry the lookup.

---

Simplify down the aliases.dts to only whats needed for the test.

- k

 libfdt/fdt_ro.c             |   21 ++++++++++++++-
 tests/Makefile.tests        |    2 +-
 tests/aliases.dts           |   21 +++++++++++++++
 tests/path_offset_aliases.c |   59 +++++++++++++++++++++++++++++++++++++++++++
 tests/run_tests.sh          |    4 +++
 5 files changed, 104 insertions(+), 3 deletions(-)
 create mode 100644 tests/aliases.dts
 create mode 100644 tests/path_offset_aliases.c
libfdt/fdt_ro.c +19 -2
index ebd1260..2f3ff48 100644
@@ -139,8 +139,25 @@ int fdt_path_offset(const void *fdt, const char *path)
 
 	FDT_CHECK_HEADER(fdt);
 
-	if (*path != '/')
-		return -FDT_ERR_BADPATH;
+	/* see if we have an alias */
+	if (*path != '/') {
+		const char *q;
+		int aliasoffset = fdt_path_offset(fdt, "/aliases");
+
+		if (aliasoffset < 0)
+			return -FDT_ERR_BADPATH;
+
+		q = strchr(path, '/');
+		if (!q)
+			q = end;
+
+		p = fdt_getprop_namelen(fdt, aliasoffset, path, q - p, NULL);
+		if (!p)
+			return -FDT_ERR_BADPATH;
+		offset = fdt_path_offset(fdt, p);
+
+		p = q;
+	}
 
 	while (*p) {
 		const char *q;
tests/Makefile.tests +1 -1
index 704c95d..44021b0 100644
@@ -11,7 +11,7 @@ LIB_TESTS_L = get_mem_rsv \
 	open_pack rw_tree1 set_name setprop del_property del_node \
 	string_escapes references path-references boot-cpuid incbin \
 	dtbs_equal_ordered \
-	add_subnode_with_nops
+	add_subnode_with_nops path_offset_aliases
 LIB_TESTS = $(LIB_TESTS_L:%=$(TESTS_PREFIX)%)
 
 LIBTREE_TESTS_L = truncated_property
tests/aliases.dts new file +21 -0
new file mode 100644
index 0000000..39d88ff
@@ -0,0 +1,21 @@
+/dts-v1/;
+
+/ {
+	aliases {
+		s1 = &sub1;
+		ss1 = &subsub1;
+		sss1 = &subsubsub1;
+	};
+
+	sub1: subnode@1 {
+		compatible = "subnode1";
+
+		subsub1: subsubnode {
+			compatible = "subsubnode1", "subsubnode";
+
+			subsubsub1: subsubsubnode {
+				compatible = "subsubsubnode1", "subsubsubnode";
+			};
+		};
+	};
+};
tests/path_offset_aliases.c new file +59 -0

59 lines not on this page · raw message

tests/run_tests.sh +4 -0
index 7bfc399..eb29462 100755
@@ -214,6 +214,10 @@ dtc_tests () {
     run_dtc_test -I dts -O dtb -o dtc_comments-cmp.test.dtb comments-cmp.dts
     run_test dtbs_equal_ordered dtc_comments.test.dtb dtc_comments-cmp.test.dtb
 
+    # Check aliases support in fdt_path_offset
+    run_dtc_test -I dts -O dtb -o aliases.dtb aliases.dts
+    run_test path_offset_aliases aliases.dtb
+
     # Check /include/ directive
     run_dtc_test -I dts -O dtb -o includes.test.dtb include0.dts
     run_test dtbs_equal_ordered includes.test.dtb test_tree1.dtb
Signature · 1 line
1.5.5.1

A message that is not in the archive (sent off-list or not yet received): Pine.LNX.4.64.0808140827220.2701-S0ohy4nlquecPGwCZ8RIISaugJOa9f/[email protected]

Acked-by
#2 lore raw
Quoted 5 lines · Kumar Gala

If the path doesn't start with '/' check to see if it matches some alias under "/aliases" and substitute the matching alias value in the path and retry the lookup.

Signature · 4 lines
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
#4 lore raw
In reply to a message not in the archive
Quoted 11 lines · Kumar Gala
libfdt/fdt_ro.c +9 -2
-	if (*path != '/')
-		return -FDT_ERR_BADPATH;
+	/* see if we have an alias */
+	if (*path != '/') {
+		const char *q;
+		int aliasoffset = fdt_path_offset(fdt, "/aliases");
+
+		if (aliasoffset < 0)
+			return -FDT_ERR_BADPATH;
+
+		q = strchr(path, '/');

libfdt/fdt_ro.c int fdt_path_offset(const void *fdt, const char *path) · line 160

Quoted 10 lines
libfdt/fdt_ro.c +10 -0
+		if (!q)
+			q = end;
+
+		p = fdt_getprop_namelen(fdt, aliasoffset, path, q - p, NULL);
+		if (!p)
+			return -FDT_ERR_BADPATH;
+		offset = fdt_path_offset(fdt, p);
+
+		p = q;
+	}

Can we limit the recursion depth to avoid falling off the stack if an alias points to itself? Or if aliases pointing to aliases are disallowed, check for a leading '/' before recursively calling fdt_path_offset.

-Scott

A message that is not in the archive (sent off-list or not yet received): 20080814174348.GA19488-VKaLA/[email protected]

#6 lore raw
Quoted 19 lines · Scott Wood
Quoted 18 lines · Kumar Gala
libfdt/fdt_ro.c +16 -2
-	if (*path != '/')
-		return -FDT_ERR_BADPATH;
+	/* see if we have an alias */
+	if (*path != '/') {
+		const char *q;
+		int aliasoffset = fdt_path_offset(fdt, "/aliases");
+
+		if (aliasoffset < 0)
+			return -FDT_ERR_BADPATH;
+
+		q = strchr(path, '/');
+		if (!q)
+			q = end;
+
+		p = fdt_getprop_namelen(fdt, aliasoffset, path, q - p, NULL);
+		if (!p)
+			return -FDT_ERR_BADPATH;
+		offset = fdt_path_offset(fdt, p);
Quoted 8 lines

libfdt/fdt_ro.c int fdt_path_offset(const void *fdt, const char *path) · line 160

Quoted 3 lines
libfdt/fdt_ro.c +3 -0
+
+		p = q;
+	}

Can we limit the recursion depth to avoid falling off the stack if an alias points to itself? Or if aliases pointing to aliases are disallowed, check for a leading '/' before recursively calling fdt_path_offset.

Hmm.. my reading of 1275 says that an alias pointing to another alias is not permitted, but I'm not terribly confident I'm not misreading it. Segher, do you know whether this is allowed?

If that's the case then, yes, we should not recurse if the alias value doesn't start with a /. In fact, if I factor out a fdt_get_alias() function, it should probably check the alias and return an error if it's not an absolute path.

Signature · 4 lines
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
#7 lore raw
Quoted 32 lines · David Gibson
Quoted 20 lines · Scott Wood
Quoted 18 lines · Kumar Gala
libfdt/fdt_ro.c +16 -2
-	if (*path != '/')
-		return -FDT_ERR_BADPATH;
+	/* see if we have an alias */
+	if (*path != '/') {
+		const char *q;
+		int aliasoffset = fdt_path_offset(fdt, "/aliases");
+
+		if (aliasoffset < 0)
+			return -FDT_ERR_BADPATH;
+
+		q = strchr(path, '/');
+		if (!q)
+			q = end;
+
+		p = fdt_getprop_namelen(fdt, aliasoffset, path, q - p, NULL);
+		if (!p)
+			return -FDT_ERR_BADPATH;
+		offset = fdt_path_offset(fdt, p);
Quoted 9 lines

libfdt/fdt_ro.c int fdt_path_offset(const void *fdt, const char *path) · line 160

Quoted 4 lines
libfdt/fdt_ro.c +3 -0
+
+		p = q;
+	}

Can we limit the recursion depth to avoid falling off the stack if an alias points to itself? Or if aliases pointing to aliases are disallowed, check for a leading '/' before recursively calling fdt_path_offset.

Quoted 4 lines

Hmm.. my reading of 1275 says that an alias pointing to another alias is not permitted, but I'm not terribly confident I'm not misreading it. Segher, do you know whether this is allowed?

The 1275 spec doesn't require multiple levels of aliasing, but my current implementation allows it and uses it for things like components of the network stack.

Quoted 6 lines

If that's the case then, yes, we should not recurse if the alias value doesn't start with a /. In fact, if I factor out a fdt_get_alias() function, it should probably check the alias and return an error if it's not an absolute path.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#9 lore raw
Quoted 3 lines

Hmm.. my reading of 1275 says that an alias pointing to another alias is not permitted, but I'm not terribly confident I'm not misreading it. Segher, do you know whether this is allowed?

My reading is the same: if after expanding an alias the path does not start with "/", the search starts at the current package, otherwise it starts at the root node; and no further alias expansion is done.

It's a corner case for sure, but at least the standard doesn't require us to implement recursive alias expansion; let's see if common practice does ;-)

Segher