linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/3] Kobjectify device tree structures
@ 2013-11-15 17:46 Grant Likely
       [not found] ` < 1384537604-5032-3-git-send-email-grant.likely@linaro.org>
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Grant Likely @ 2013-11-15 17:46 UTC (permalink / raw)
  To: devicetree, linux-kernel

Hi all,

I've resurrected this series and am posting it for review. I think I've
got the last problem that I know about solved. That was the problem that
Nathan pointed out that modified properties don't get reflected in the
sysfs representation. As I mentioned the last time I posted this series,
I will probably put a kernel release between patches 1&2 and patch 3 so
that if there is any problems then the old /proc method is still
available.

I'm posting this for review, but I've only compile tested on ARM and
PowerPC, and boot tested on ARM. I'll be going though a full set of
builds when I'm no longer on an airplane.  :-)

The existing device-tree userspace ABI remains the same with this series
applied.

Nathan, I would particularly appreciate if you can test this on PowerPC
machines with OF_DYNAMIC.

g.


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

* [PATCH v5 1/3] of: Make device nodes kobjects so they show up in sysfs
  2013-11-15 17:46 [PATCH v5 0/3] Kobjectify device tree structures Grant Likely
       [not found] ` < 1384537604-5032-3-git-send-email-grant.likely@linaro.org>
@ 2013-11-15 17:46 ` Grant Likely
  2013-11-15 17:46 ` [PATCH v5 2/3] of/selftest: Add self tests for manipulation of properties Grant Likely
  2013-11-15 17:46 ` [PATCH v5 3/3] of: remove /proc/device-tree Grant Likely
  3 siblings, 0 replies; 8+ messages in thread
From: Grant Likely @ 2013-11-15 17:46 UTC (permalink / raw)
  To: devicetree, linux-kernel
  Cc: Grant Likely, Rob Herring, Benjamin Herrenschmidt,
	David S. Miller, Nathan Fontenot, Pantelis Antoniou

From: Grant Likely <grant.likely@secretlab.ca>

Device tree nodes are already treated as objects, and we already want to
expose them to userspace which is done using the /proc filesystem today.
Right now the kernel has to do a lot of work to keep the /proc view in
sync with the in-kernel representation. If device_nodes are switched to
be kobjects then the device tree code can be a whole lot simpler. It
also turns out that switching to using /sysfs from /proc results in
smaller code and data size, and the userspace ABI won't change if
/proc/device-tree symlinks to /sys/firmware/devicetree/base.

v5: Rename firmware/ofw to firmware/devicetree
    Fix updating property values in sysfs
v4: Fixed build error on Powerpc
    Fixed handling of dynamic nodes on powerpc
v3: Fixed handling of duplicate attribute and child node names
v2: switch to using sysfs bin_attributes which solve the problem of
    reporting incorrect property size.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Tested-by: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 Documentation/ABI/testing/sysfs-firmware-ofw   |  28 +++++
 arch/arm/boot/dts/testcases/tests-phandle.dtsi |   3 +
 arch/powerpc/platforms/pseries/dlpar.c         |   1 -
 arch/powerpc/platforms/pseries/reconfig.c      |   2 -
 arch/powerpc/sysdev/msi_bitmap.c               |   2 +-
 drivers/of/base.c                              | 157 +++++++++++++++++++++++--
 drivers/of/fdt.c                               |   3 +-
 drivers/of/pdt.c                               |   4 +-
 include/linux/of.h                             |   9 +-
 9 files changed, 193 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-firmware-ofw

diff --git a/Documentation/ABI/testing/sysfs-firmware-ofw b/Documentation/ABI/testing/sysfs-firmware-ofw
new file mode 100644
index 000000000000..f562b188e71d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-firmware-ofw
@@ -0,0 +1,28 @@
+What:		/sys/firmware/devicetree/*
+Date:		November 2013
+Contact:	Grant Likely <grant.likely@linaro.org>
+Description:
+		When using OpenFirmware or a Flattened Device Tree to enumerate
+		hardware, the device tree structure will be exposed in this
+		directory.
+
+		It is possible for multiple device-tree directories to exist.
+		Some device drivers use a separate detached device tree which
+		have no attachment to the system tree and will appear in a
+		different subdirectory under /sys/firmware/devicetree.
+
+		Userspace must not use the /sys/firmware/devicetree/base
+		path directly, but instead should follow /proc/device-tree
+		symlink. It is possible that the absolute path will change
+		in the future, but the symlink is the stable ABI.
+
+		The /proc/device-tree symlink replaces the devicetree /proc
+		filesystem support, and has largely the same semantics and
+		should be compatible with existing userspace.
+
+		The contents of /sys/firmware/devicetree/ is a
+		hierarchy of directories, one per device tree node. The
+		directory name is the resolved path component name (node
+		name plus address). Properties are represented as files
+		in the directory. The contents of each file is the exact
+		binary data from the device tree.
diff --git a/arch/arm/boot/dts/testcases/tests-phandle.dtsi b/arch/arm/boot/dts/testcases/tests-phandle.dtsi
index 0007d3cd7dc2..788a4c24b8f5 100644
--- a/arch/arm/boot/dts/testcases/tests-phandle.dtsi
+++ b/arch/arm/boot/dts/testcases/tests-phandle.dtsi
@@ -1,6 +1,9 @@
 
 / {
 	testcase-data {
+		security-password = "password";
+		duplicate-name = "duplicate";
+		duplicate-name { };
 		phandle-tests {
 			provider0: provider0 {
 				#phandle-cells = <0>;
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index a8fe5aa3d34f..397661de19ec 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -11,7 +11,6 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/kref.h>
 #include <linux/notifier.h>
 #include <linux/spinlock.h>
 #include <linux/cpu.h>
diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
index f93cdf55628c..0435bb65d0aa 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -12,7 +12,6 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/kref.h>
 #include <linux/notifier.h>
 #include <linux/proc_fs.h>
 #include <linux/slab.h>
@@ -70,7 +69,6 @@ static int pSeries_reconfig_add_node(const char *path, struct property *proplist
 
 	np->properties = proplist;
 	of_node_set_flag(np, OF_DYNAMIC);
-	kref_init(&np->kref);
 
 	np->parent = derive_parent(path);
 	if (IS_ERR(np->parent)) {
diff --git a/arch/powerpc/sysdev/msi_bitmap.c b/arch/powerpc/sysdev/msi_bitmap.c
index 0968b66b4cf9..8ba60424be95 100644
--- a/arch/powerpc/sysdev/msi_bitmap.c
+++ b/arch/powerpc/sysdev/msi_bitmap.c
@@ -202,7 +202,7 @@ void __init test_of_node(void)
 
 	/* There should really be a struct device_node allocator */
 	memset(&of_node, 0, sizeof(of_node));
-	kref_init(&of_node.kref);
+	kref_init(&of_node.kobj.kref);
 	of_node.full_name = node_name;
 
 	check(0 == msi_bitmap_alloc(&bmp, size, &of_node));
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 967b3209907a..efcfead5ba2d 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -23,6 +23,7 @@
 #include <linux/of.h>
 #include <linux/spinlock.h>
 #include <linux/slab.h>
+#include <linux/string.h>
 #include <linux/proc_fs.h>
 
 #include "of_private.h"
@@ -35,6 +36,12 @@ struct device_node *of_chosen;
 struct device_node *of_aliases;
 static struct device_node *of_stdout;
 
+static struct kset *of_kset;
+
+/*
+ * Used to protect the of_aliases; but also overloaded to hold off addition of
+ * nodes to sysfs
+ */
 DEFINE_MUTEX(of_aliases_mutex);
 
 /* use when traversing tree through the allnext, child, sibling,
@@ -92,14 +99,14 @@ int __weak of_node_to_nid(struct device_node *np)
 struct device_node *of_node_get(struct device_node *node)
 {
 	if (node)
-		kref_get(&node->kref);
+		kobject_get(&node->kobj);
 	return node;
 }
 EXPORT_SYMBOL(of_node_get);
 
-static inline struct device_node *kref_to_device_node(struct kref *kref)
+static inline struct device_node *kobj_to_device_node(struct kobject *kobj)
 {
-	return container_of(kref, struct device_node, kref);
+	return container_of(kobj, struct device_node, kobj);
 }
 
 /**
@@ -109,16 +116,15 @@ static inline struct device_node *kref_to_device_node(struct kref *kref)
  *	In of_node_put() this function is passed to kref_put()
  *	as the destructor.
  */
-static void of_node_release(struct kref *kref)
+static void of_node_release(struct kobject *kobj)
 {
-	struct device_node *node = kref_to_device_node(kref);
+	struct device_node *node = kobj_to_device_node(kobj);
 	struct property *prop = node->properties;
 
 	/* We should never be releasing nodes that haven't been detached. */
 	if (!of_node_check_flag(node, OF_DETACHED)) {
 		pr_err("ERROR: Bad of_node_put() on %s\n", node->full_name);
 		dump_stack();
-		kref_init(&node->kref);
 		return;
 	}
 
@@ -151,11 +157,138 @@ static void of_node_release(struct kref *kref)
 void of_node_put(struct device_node *node)
 {
 	if (node)
-		kref_put(&node->kref, of_node_release);
+		kobject_put(&node->kobj);
 }
 EXPORT_SYMBOL(of_node_put);
+#else
+static void of_node_release(struct kobject *kobj)
+{
+	/* Without CONFIG_OF_DYNAMIC, no nodes gets freed */
+}
 #endif /* CONFIG_OF_DYNAMIC */
 
+struct kobj_type of_node_ktype = {
+	.release = of_node_release,
+};
+
+static ssize_t of_node_property_read(struct file *filp, struct kobject *kobj,
+				struct bin_attribute *bin_attr, char *buf,
+				loff_t offset, size_t count)
+{
+	struct property *pp = container_of(bin_attr, struct property, attr);
+	return memory_read_from_buffer(buf, count, &offset, pp->value, pp->length);
+}
+
+static const char *safe_name(struct kobject *kobj, const char *orig_name)
+{
+	const char *name = orig_name;
+	struct sysfs_dirent *sd;
+	int i = 0;
+
+	/* don't be a hero. After 16 tries give up */
+	while (i < 16 && (sd = sysfs_get_dirent(kobj->sd, name))) {
+		sysfs_put(sd);
+		if (name != orig_name)
+			kfree(name);
+		name = kasprintf(GFP_KERNEL, "%s#%i", orig_name, ++i);
+	}
+
+	if (name != orig_name)
+		pr_warn("device-tree: Duplicate name in %s, renamed to \"%s\"\n",
+			kobject_name(kobj), name);
+	return name;
+}
+
+static int __of_add_property(struct device_node *np, struct property *pp)
+{
+	int rc;
+
+	/* Important: Don't leak passwords */
+	bool secure = strncmp(pp->name, "security-", 9) == 0;
+
+	pp->attr.attr.name = safe_name(&np->kobj, pp->name);
+	pp->attr.attr.mode = secure ? S_IRUSR : S_IRUGO;
+	pp->attr.size = secure ? 0 : pp->length;
+	pp->attr.read = of_node_property_read;
+
+	rc = sysfs_create_bin_file(&np->kobj, &pp->attr);
+	WARN(rc, "error adding attribute %s to node %s\n", pp->name, np->full_name);
+	return rc;
+}
+
+static int __of_node_add(struct device_node *np)
+{
+	const char *name;
+	struct property *pp;
+	int rc;
+
+	np->kobj.kset = of_kset;
+	if (!np->parent) {
+		/* Nodes without parents are new top level trees */
+		rc = kobject_add(&np->kobj, NULL, safe_name(&of_kset->kobj, "base"));
+	} else {
+		name = safe_name(&np->parent->kobj, kbasename(np->full_name));
+		if (!name || !name[0])
+			return -EINVAL;
+
+		rc = kobject_add(&np->kobj, &np->parent->kobj, "%s", name);
+	}
+	if (rc)
+		return rc;
+
+	for_each_property_of_node(np, pp)
+		__of_add_property(np, pp);
+
+	return 0;
+}
+
+int of_node_add(struct device_node *np)
+{
+	int rc = 0;
+	kobject_init(&np->kobj, &of_node_ktype);
+	mutex_lock(&of_aliases_mutex);
+	if (of_kset)
+		rc = __of_node_add(np);
+	mutex_unlock(&of_aliases_mutex);
+	return rc;
+}
+
+#if defined(CONFIG_OF_DYNAMIC)
+static void of_node_remove(struct device_node *np)
+{
+	struct property *pp;
+
+	for_each_property_of_node(np, pp)
+		sysfs_remove_bin_file(&np->kobj, &pp->attr);
+
+	kobject_del(&np->kobj);
+}
+#endif
+
+static int __init of_init(void)
+{
+	struct device_node *np;
+
+	of_kset = kset_create_and_add("devicetree", NULL, firmware_kobj);
+	if (!of_kset)
+		return -ENOMEM;
+
+	/* Make sure all nodes added before this time get added to sysfs */
+	mutex_lock(&of_aliases_mutex);
+	for_each_of_allnodes(np)
+		__of_node_add(np);
+	mutex_unlock(&of_aliases_mutex);
+
+#if !defined(CONFIG_PROC_DEVICETREE)
+	/* Symlink to the new tree when PROC_DEVICETREE is disabled */
+	if (of_allnodes)
+		proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
+#endif /* CONFIG_PROC_DEVICETREE */
+
+	return 0;
+}
+core_initcall(of_init);
+
 static struct property *__of_find_property(const struct device_node *np,
 					   const char *name, int *lenp)
 {
@@ -1495,6 +1628,8 @@ int of_add_property(struct device_node *np, struct property *prop)
 	*next = prop;
 	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 
+	__of_add_property(np, prop);
+
 #ifdef CONFIG_PROC_DEVICETREE
 	/* try to add to proc as well if it was initialized */
 	if (np->pde)
@@ -1541,6 +1676,8 @@ int of_remove_property(struct device_node *np, struct property *prop)
 	if (!found)
 		return -ENODEV;
 
+	sysfs_remove_bin_file(&np->kobj, &prop->attr);
+
 #ifdef CONFIG_PROC_DEVICETREE
 	/* try to remove the proc node as well */
 	if (np->pde)
@@ -1595,6 +1732,10 @@ int of_update_property(struct device_node *np, struct property *newprop)
 	if (!found)
 		return -ENODEV;
 
+	/* Update the sysfs attribute */
+	sysfs_remove_bin_file(&np->kobj, &oldprop->attr);
+	__of_add_property(np, newprop);
+
 #ifdef CONFIG_PROC_DEVICETREE
 	/* try to add to proc as well if it was initialized */
 	if (np->pde)
@@ -1671,6 +1812,7 @@ int of_attach_node(struct device_node *np)
 	of_node_clear_flag(np, OF_DETACHED);
 	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 
+	of_node_add(np);
 	of_add_proc_dt_entry(np);
 	return 0;
 }
@@ -1743,6 +1885,7 @@ int of_detach_node(struct device_node *np)
 	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 
 	of_remove_proc_dt_entry(np);
+	of_node_remove(np);
 	return rc;
 }
 #endif /* defined(CONFIG_OF_DYNAMIC) */
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 2fa024b97c43..d5d62cd5680f 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -232,7 +232,6 @@ static void * unflatten_dt_node(struct boot_param_header *blob,
 				dad->next->sibling = np;
 			dad->next = np;
 		}
-		kref_init(&np->kref);
 	}
 	/* process properties */
 	while (1) {
@@ -327,6 +326,8 @@ static void * unflatten_dt_node(struct boot_param_header *blob,
 			np->name = "<NULL>";
 		if (!np->type)
 			np->type = "<NULL>";
+
+		of_node_add(np);
 	}
 	while (tag == OF_DT_BEGIN_NODE || tag == OF_DT_NOP) {
 		if (tag == OF_DT_NOP)
diff --git a/drivers/of/pdt.c b/drivers/of/pdt.c
index 7b666736c168..e64fa3d3da5f 100644
--- a/drivers/of/pdt.c
+++ b/drivers/of/pdt.c
@@ -179,8 +179,6 @@ static struct device_node * __init of_pdt_create_node(phandle node,
 	of_pdt_incr_unique_id(dp);
 	dp->parent = parent;
 
-	kref_init(&dp->kref);
-
 	dp->name = of_pdt_get_one_property(node, "name");
 	dp->type = of_pdt_get_one_property(node, "device_type");
 	dp->phandle = node;
@@ -215,6 +213,7 @@ static struct device_node * __init of_pdt_build_tree(struct device_node *parent,
 		*nextp = &dp->allnext;
 
 		dp->full_name = of_pdt_build_full_name(dp);
+		of_node_add(dp);
 
 		dp->child = of_pdt_build_tree(dp,
 				of_pdt_prom_ops->getchild(node), nextp);
@@ -245,6 +244,7 @@ void __init of_pdt_build_devicetree(phandle root_node, struct of_pdt_ops *ops)
 	of_allnodes->path_component_name = "";
 #endif
 	of_allnodes->full_name = "/";
+	of_node_add(of_allnodes);
 
 	nextp = &of_allnodes->allnext;
 	of_allnodes->child = of_pdt_build_tree(of_allnodes,
diff --git a/include/linux/of.h b/include/linux/of.h
index a4b7fdf17044..c899a89de9a8 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -18,7 +18,7 @@
 #include <linux/types.h>
 #include <linux/bitops.h>
 #include <linux/errno.h>
-#include <linux/kref.h>
+#include <linux/kobject.h>
 #include <linux/mod_devicetable.h>
 #include <linux/spinlock.h>
 #include <linux/topology.h>
@@ -37,6 +37,7 @@ struct property {
 	struct property *next;
 	unsigned long _flags;
 	unsigned int unique_id;
+	struct bin_attribute attr;
 };
 
 #if defined(CONFIG_SPARC)
@@ -57,7 +58,7 @@ struct device_node {
 	struct	device_node *next;	/* next device of same type */
 	struct	device_node *allnext;	/* next in list of all nodes */
 	struct	proc_dir_entry *pde;	/* this node's proc directory */
-	struct	kref kref;
+	struct	kobject kobj;
 	unsigned long _flags;
 	void	*data;
 #if defined(CONFIG_SPARC)
@@ -74,6 +75,8 @@ struct of_phandle_args {
 	uint32_t args[MAX_PHANDLE_ARGS];
 };
 
+extern int of_node_add(struct device_node *node);
+
 #ifdef CONFIG_OF_DYNAMIC
 extern struct device_node *of_node_get(struct device_node *node);
 extern void of_node_put(struct device_node *node);
@@ -187,6 +190,8 @@ static inline const char *of_node_full_name(const struct device_node *np)
 	return np ? np->full_name : "<no-node>";
 }
 
+#define for_each_of_allnodes(dn) \
+	for (dn = of_allnodes; dn; dn = dn->allnext)
 extern struct device_node *of_find_node_by_name(struct device_node *from,
 	const char *name);
 #define for_each_node_by_name(dn, name) \
-- 
1.8.3.2


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

* [PATCH v5 2/3] of/selftest: Add self tests for manipulation of properties
  2013-11-15 17:46 [PATCH v5 0/3] Kobjectify device tree structures Grant Likely
       [not found] ` < 1384537604-5032-3-git-send-email-grant.likely@linaro.org>
  2013-11-15 17:46 ` [PATCH v5 1/3] of: Make device nodes kobjects so they show up in sysfs Grant Likely
@ 2013-11-15 17:46 ` Grant Likely
  2013-11-16 17:50   ` Pantelis Antoniou
  2013-11-16 19:09   ` Rob Herring
  2013-11-15 17:46 ` [PATCH v5 3/3] of: remove /proc/device-tree Grant Likely
  3 siblings, 2 replies; 8+ messages in thread
From: Grant Likely @ 2013-11-15 17:46 UTC (permalink / raw)
  To: devicetree, linux-kernel
  Cc: Grant Likely, Rob Herring, Benjamin Herrenschmidt,
	David S. Miller, Nathan Fontenot, Pantelis Antoniou

Adds a few simple test cases to ensure that addition, update and removal
of device tree node properties works correctly.

Signed-off-by: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/of/selftest.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index e21012bde639..cb8d3e722f76 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -30,6 +30,67 @@ static struct selftest_results {
 	} \
 }
 
+static void __init of_selftest_dynamic(void)
+{
+	struct device_node *np;
+	struct property *prop;
+
+	np = of_find_node_by_path("/testcase-data");
+	if (!np) {
+		pr_err("missing testcase data\n");
+		return;
+	}
+
+	/* Array of 4 properties for the purpose of testing */
+	prop = kzalloc(sizeof(*prop) * 4, GFP_KERNEL);
+	if (!prop) {
+		selftest(0, "kzalloc() failed\n");
+		return;
+	}
+
+	/* Add a new property - should pass*/
+	prop->name = "new-property";
+	prop->value = "new-property-data";
+	prop->length = strlen(prop->value);
+	selftest(of_add_property(np, prop) == 0, "Adding a new property failed\n");
+
+	/* Try to add an existing property - should fail */
+	prop++;
+	prop->name = "new-property";
+	prop->value = "new-property-data-should-fail";
+	prop->length = strlen(prop->value);
+	selftest(of_add_property(np, prop) != 0,
+		 "Adding an existing property should have failed\n");
+
+	/* Try to modify an existing property - should pass */
+	prop->value = "modify-property-data-should-pass";
+	prop->length = strlen(prop->value);
+	selftest(of_update_property(np, prop) == 0,
+		 "Updating an existing property should have passed\n");
+
+	/* Try to modify non-existent property - should pass*/
+	prop++;
+	prop->name = "modify-property";
+	prop->value = "modify-missing-property-data-should-pass";
+	prop->length = strlen(prop->value);
+	selftest(of_update_property(np, prop) == 0,
+		 "Updating a missing property should have passed\n");
+
+	/* Remove property - should pass */
+	selftest(of_remove_property(np, prop) == 0,
+		 "Removing a property should have passed\n");
+
+	/* Adding very large property - should pass */
+	prop++;
+	prop->name = "large-property-PAGE_SIZEx8";
+	prop->length = PAGE_SIZE * 8;
+	prop->value = kzalloc(prop->length, GFP_KERNEL);
+	selftest(prop->value != NULL, "Unable to allocate large buffer\n");
+	if (prop->value)
+		selftest(of_add_property(np, prop) == 0,
+			 "Adding a large property should have passed\n");
+}
+
 static void __init of_selftest_parse_phandle_with_args(void)
 {
 	struct device_node *np;
@@ -312,6 +373,7 @@ static int __init of_selftest(void)
 	of_node_put(np);
 
 	pr_info("start of selftest - you will see error messages\n");
+	of_selftest_dynamic();
 	of_selftest_parse_phandle_with_args();
 	of_selftest_property_match_string();
 	of_selftest_parse_interrupts();
-- 
1.8.3.2


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

* [PATCH v5 3/3] of: remove /proc/device-tree
  2013-11-15 17:46 [PATCH v5 0/3] Kobjectify device tree structures Grant Likely
                   ` (2 preceding siblings ...)
  2013-11-15 17:46 ` [PATCH v5 2/3] of/selftest: Add self tests for manipulation of properties Grant Likely
@ 2013-11-15 17:46 ` Grant Likely
  3 siblings, 0 replies; 8+ messages in thread
From: Grant Likely @ 2013-11-15 17:46 UTC (permalink / raw)
  To: devicetree, linux-kernel
  Cc: Grant Likely, Rob Herring, Benjamin Herrenschmidt,
	David S. Miller, Nathan Fontenot, Pantelis Antoniou

From: Grant Likely <grant.likely@secretlab.ca>

The same data is now available in sysfs, so we can remove the code
that exports it in /proc and replace it with a symlink to the sysfs
version.

Tested on versatile qemu model and mpc5200 eval board. More testing
would be appreciated.

v5: Fixed up conflicts with mainline changes

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/of/Kconfig     |   8 --
 drivers/of/base.c      |  52 +----------
 fs/proc/Makefile       |   1 -
 fs/proc/internal.h     |   7 --
 fs/proc/proc_devtree.c | 240 -------------------------------------------------
 fs/proc/root.c         |   3 -
 include/linux/of.h     |  11 ---
 7 files changed, 1 insertion(+), 321 deletions(-)
 delete mode 100644 fs/proc/proc_devtree.c

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index de6f8990246f..46e211e2bc51 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -7,14 +7,6 @@ config OF
 menu "Device Tree and Open Firmware support"
 	depends on OF
 
-config PROC_DEVICETREE
-	bool "Support for device tree in /proc"
-	depends on PROC_FS && !SPARC
-	help
-	  This option adds a device-tree directory under /proc which contains
-	  an image of the device tree that the kernel copies from Open
-	  Firmware or other boot firmware. If unsure, say Y here.
-
 config OF_SELFTEST
 	bool "Device Tree Runtime self tests"
 	depends on OF_IRQ
diff --git a/drivers/of/base.c b/drivers/of/base.c
index efcfead5ba2d..4f76d5ff53aa 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -279,11 +279,9 @@ static int __init of_init(void)
 		__of_node_add(np);
 	mutex_unlock(&of_aliases_mutex);
 
-#if !defined(CONFIG_PROC_DEVICETREE)
-	/* Symlink to the new tree when PROC_DEVICETREE is disabled */
+	/* Symlink in /proc as required by userspace ABI */
 	if (of_allnodes)
 		proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
-#endif /* CONFIG_PROC_DEVICETREE */
 
 	return 0;
 }
@@ -1630,12 +1628,6 @@ int of_add_property(struct device_node *np, struct property *prop)
 
 	__of_add_property(np, prop);
 
-#ifdef CONFIG_PROC_DEVICETREE
-	/* try to add to proc as well if it was initialized */
-	if (np->pde)
-		proc_device_tree_add_prop(np->pde, prop);
-#endif /* CONFIG_PROC_DEVICETREE */
-
 	return 0;
 }
 
@@ -1678,12 +1670,6 @@ int of_remove_property(struct device_node *np, struct property *prop)
 
 	sysfs_remove_bin_file(&np->kobj, &prop->attr);
 
-#ifdef CONFIG_PROC_DEVICETREE
-	/* try to remove the proc node as well */
-	if (np->pde)
-		proc_device_tree_remove_prop(np->pde, prop);
-#endif /* CONFIG_PROC_DEVICETREE */
-
 	return 0;
 }
 
@@ -1736,12 +1722,6 @@ int of_update_property(struct device_node *np, struct property *newprop)
 	sysfs_remove_bin_file(&np->kobj, &oldprop->attr);
 	__of_add_property(np, newprop);
 
-#ifdef CONFIG_PROC_DEVICETREE
-	/* try to add to proc as well if it was initialized */
-	if (np->pde)
-		proc_device_tree_update_prop(np->pde, newprop, oldprop);
-#endif /* CONFIG_PROC_DEVICETREE */
-
 	return 0;
 }
 
@@ -1776,22 +1756,6 @@ int of_reconfig_notify(unsigned long action, void *p)
 	return notifier_to_errno(rc);
 }
 
-#ifdef CONFIG_PROC_DEVICETREE
-static void of_add_proc_dt_entry(struct device_node *dn)
-{
-	struct proc_dir_entry *ent;
-
-	ent = proc_mkdir(strrchr(dn->full_name, '/') + 1, dn->parent->pde);
-	if (ent)
-		proc_device_tree_add_node(dn, ent);
-}
-#else
-static void of_add_proc_dt_entry(struct device_node *dn)
-{
-	return;
-}
-#endif
-
 /**
  * of_attach_node - Plug a device node into the tree and global list.
  */
@@ -1813,22 +1777,9 @@ int of_attach_node(struct device_node *np)
 	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 
 	of_node_add(np);
-	of_add_proc_dt_entry(np);
 	return 0;
 }
 
-#ifdef CONFIG_PROC_DEVICETREE
-static void of_remove_proc_dt_entry(struct device_node *dn)
-{
-	proc_remove(dn->pde);
-}
-#else
-static void of_remove_proc_dt_entry(struct device_node *dn)
-{
-	return;
-}
-#endif
-
 /**
  * of_detach_node - "Unplug" a node from the device tree.
  *
@@ -1884,7 +1835,6 @@ int of_detach_node(struct device_node *np)
 	of_node_set_flag(np, OF_DETACHED);
 	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 
-	of_remove_proc_dt_entry(np);
 	of_node_remove(np);
 	return rc;
 }
diff --git a/fs/proc/Makefile b/fs/proc/Makefile
index ab30716584f5..239493ec718e 100644
--- a/fs/proc/Makefile
+++ b/fs/proc/Makefile
@@ -27,6 +27,5 @@ proc-$(CONFIG_PROC_SYSCTL)	+= proc_sysctl.o
 proc-$(CONFIG_NET)		+= proc_net.o
 proc-$(CONFIG_PROC_KCORE)	+= kcore.o
 proc-$(CONFIG_PROC_VMCORE)	+= vmcore.o
-proc-$(CONFIG_PROC_DEVICETREE)	+= proc_devtree.o
 proc-$(CONFIG_PRINTK)	+= kmsg.o
 proc-$(CONFIG_PROC_PAGE_MONITOR)	+= page.o
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 651d09a11dde..3ab6d14e71c5 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -211,13 +211,6 @@ extern int proc_fill_super(struct super_block *);
 extern void proc_entry_rundown(struct proc_dir_entry *);
 
 /*
- * proc_devtree.c
- */
-#ifdef CONFIG_PROC_DEVICETREE
-extern void proc_device_tree_init(void);
-#endif
-
-/*
  * proc_namespaces.c
  */
 extern const struct inode_operations proc_ns_dir_inode_operations;
diff --git a/fs/proc/proc_devtree.c b/fs/proc/proc_devtree.c
deleted file mode 100644
index 70779b2fc209..000000000000
--- a/fs/proc/proc_devtree.c
+++ /dev/null
@@ -1,240 +0,0 @@
-/*
- * proc_devtree.c - handles /proc/device-tree
- *
- * Copyright 1997 Paul Mackerras
- */
-#include <linux/errno.h>
-#include <linux/init.h>
-#include <linux/time.h>
-#include <linux/proc_fs.h>
-#include <linux/seq_file.h>
-#include <linux/printk.h>
-#include <linux/stat.h>
-#include <linux/string.h>
-#include <linux/of.h>
-#include <linux/export.h>
-#include <linux/slab.h>
-#include <asm/uaccess.h>
-#include "internal.h"
-
-static inline void set_node_proc_entry(struct device_node *np,
-				       struct proc_dir_entry *de)
-{
-	np->pde = de;
-}
-
-static struct proc_dir_entry *proc_device_tree;
-
-/*
- * Supply data on a read from /proc/device-tree/node/property.
- */
-static int property_proc_show(struct seq_file *m, void *v)
-{
-	struct property *pp = m->private;
-
-	seq_write(m, pp->value, pp->length);
-	return 0;
-}
-
-static int property_proc_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, property_proc_show, __PDE_DATA(inode));
-}
-
-static const struct file_operations property_proc_fops = {
-	.owner		= THIS_MODULE,
-	.open		= property_proc_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
-
-/*
- * For a node with a name like "gc@10", we make symlinks called "gc"
- * and "@10" to it.
- */
-
-/*
- * Add a property to a node
- */
-static struct proc_dir_entry *
-__proc_device_tree_add_prop(struct proc_dir_entry *de, struct property *pp,
-		const char *name)
-{
-	struct proc_dir_entry *ent;
-
-	/*
-	 * Unfortunately proc_register puts each new entry
-	 * at the beginning of the list.  So we rearrange them.
-	 */
-	ent = proc_create_data(name,
-			       strncmp(name, "security-", 9) ? S_IRUGO : S_IRUSR,
-			       de, &property_proc_fops, pp);
-	if (ent == NULL)
-		return NULL;
-
-	if (!strncmp(name, "security-", 9))
-		ent->size = 0; /* don't leak number of password chars */
-	else
-		ent->size = pp->length;
-
-	return ent;
-}
-
-
-void proc_device_tree_add_prop(struct proc_dir_entry *pde, struct property *prop)
-{
-	__proc_device_tree_add_prop(pde, prop, prop->name);
-}
-
-void proc_device_tree_remove_prop(struct proc_dir_entry *pde,
-				  struct property *prop)
-{
-	remove_proc_entry(prop->name, pde);
-}
-
-void proc_device_tree_update_prop(struct proc_dir_entry *pde,
-				  struct property *newprop,
-				  struct property *oldprop)
-{
-	struct proc_dir_entry *ent;
-
-	if (!oldprop) {
-		proc_device_tree_add_prop(pde, newprop);
-		return;
-	}
-
-	for (ent = pde->subdir; ent != NULL; ent = ent->next)
-		if (ent->data == oldprop)
-			break;
-	if (ent == NULL) {
-		pr_warn("device-tree: property \"%s\" does not exist\n",
-			oldprop->name);
-	} else {
-		ent->data = newprop;
-		ent->size = newprop->length;
-	}
-}
-
-/*
- * Various dodgy firmware might give us nodes and/or properties with
- * conflicting names. That's generally ok, except for exporting via /proc,
- * so munge names here to ensure they're unique.
- */
-
-static int duplicate_name(struct proc_dir_entry *de, const char *name)
-{
-	struct proc_dir_entry *ent;
-	int found = 0;
-
-	spin_lock(&proc_subdir_lock);
-
-	for (ent = de->subdir; ent != NULL; ent = ent->next) {
-		if (strcmp(ent->name, name) == 0) {
-			found = 1;
-			break;
-		}
-	}
-
-	spin_unlock(&proc_subdir_lock);
-
-	return found;
-}
-
-static const char *fixup_name(struct device_node *np, struct proc_dir_entry *de,
-		const char *name)
-{
-	char *fixed_name;
-	int fixup_len = strlen(name) + 2 + 1; /* name + #x + \0 */
-	int i = 1, size;
-
-realloc:
-	fixed_name = kmalloc(fixup_len, GFP_KERNEL);
-	if (fixed_name == NULL) {
-		pr_err("device-tree: Out of memory trying to fixup "
-		       "name \"%s\"\n", name);
-		return name;
-	}
-
-retry:
-	size = snprintf(fixed_name, fixup_len, "%s#%d", name, i);
-	size++; /* account for NULL */
-
-	if (size > fixup_len) {
-		/* We ran out of space, free and reallocate. */
-		kfree(fixed_name);
-		fixup_len = size;
-		goto realloc;
-	}
-
-	if (duplicate_name(de, fixed_name)) {
-		/* Multiple duplicates. Retry with a different offset. */
-		i++;
-		goto retry;
-	}
-
-	pr_warn("device-tree: Duplicate name in %s, renamed to \"%s\"\n",
-		np->full_name, fixed_name);
-
-	return fixed_name;
-}
-
-/*
- * Process a node, adding entries for its children and its properties.
- */
-void proc_device_tree_add_node(struct device_node *np,
-			       struct proc_dir_entry *de)
-{
-	struct property *pp;
-	struct proc_dir_entry *ent;
-	struct device_node *child;
-	const char *p;
-
-	set_node_proc_entry(np, de);
-	for (child = NULL; (child = of_get_next_child(np, child));) {
-		/* Use everything after the last slash, or the full name */
-		p = kbasename(child->full_name);
-
-		if (duplicate_name(de, p))
-			p = fixup_name(np, de, p);
-
-		ent = proc_mkdir(p, de);
-		if (ent == NULL)
-			break;
-		proc_device_tree_add_node(child, ent);
-	}
-	of_node_put(child);
-
-	for (pp = np->properties; pp != NULL; pp = pp->next) {
-		p = pp->name;
-
-		if (strchr(p, '/'))
-			continue;
-
-		if (duplicate_name(de, p))
-			p = fixup_name(np, de, p);
-
-		ent = __proc_device_tree_add_prop(de, pp, p);
-		if (ent == NULL)
-			break;
-	}
-}
-
-/*
- * Called on initialization to set up the /proc/device-tree subtree
- */
-void __init proc_device_tree_init(void)
-{
-	struct device_node *root;
-
-	proc_device_tree = proc_mkdir("device-tree", NULL);
-	if (proc_device_tree == NULL)
-		return;
-	root = of_find_node_by_path("/");
-	if (root == NULL) {
-		pr_debug("/proc/device-tree: can't find root\n");
-		return;
-	}
-	proc_device_tree_add_node(root, proc_device_tree);
-	of_node_put(root);
-}
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 87dbcbef7fe4..7bbeb5257af1 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -183,9 +183,6 @@ void __init proc_root_init(void)
 	proc_mkdir("openprom", NULL);
 #endif
 	proc_tty_init();
-#ifdef CONFIG_PROC_DEVICETREE
-	proc_device_tree_init();
-#endif
 	proc_mkdir("bus", NULL);
 	proc_sys_init();
 }
diff --git a/include/linux/of.h b/include/linux/of.h
index c899a89de9a8..06d32ad62bc0 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -57,7 +57,6 @@ struct device_node {
 	struct	device_node *sibling;
 	struct	device_node *next;	/* next device of same type */
 	struct	device_node *allnext;	/* next in list of all nodes */
-	struct	proc_dir_entry *pde;	/* this node's proc directory */
 	struct	kobject kobj;
 	unsigned long _flags;
 	void	*data;
@@ -638,14 +637,4 @@ static inline int of_property_read_u32(const struct device_node *np,
 		s;						\
 		s = of_prop_next_string(prop, s))
 
-#if defined(CONFIG_PROC_FS) && defined(CONFIG_PROC_DEVICETREE)
-extern void proc_device_tree_add_node(struct device_node *, struct proc_dir_entry *);
-extern void proc_device_tree_add_prop(struct proc_dir_entry *pde, struct property *prop);
-extern void proc_device_tree_remove_prop(struct proc_dir_entry *pde,
-					 struct property *prop);
-extern void proc_device_tree_update_prop(struct proc_dir_entry *pde,
-					 struct property *newprop,
-					 struct property *oldprop);
-#endif
-
 #endif /* _LINUX_OF_H */
-- 
1.8.3.2


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

* Re: [PATCH v5 2/3] of/selftest: Add self tests for manipulation of properties
  2013-11-15 17:46 ` [PATCH v5 2/3] of/selftest: Add self tests for manipulation of properties Grant Likely
@ 2013-11-16 17:50   ` Pantelis Antoniou
  2013-11-17 22:14     ` Grant Likely
  2013-11-16 19:09   ` Rob Herring
  1 sibling, 1 reply; 8+ messages in thread
From: Pantelis Antoniou @ 2013-11-16 17:50 UTC (permalink / raw)
  To: Grant Likely
  Cc: devicetree, linux-kernel, Rob Herring, Benjamin Herrenschmidt,
	David S. Miller, Nathan Fontenot

Hi Grant,

On Nov 15, 2013, at 7:46 PM, Grant Likely wrote:

> Adds a few simple test cases to ensure that addition, update and removal
> of device tree node properties works correctly.
> 
> Signed-off-by: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
> drivers/of/selftest.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
> 
> diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
> index e21012bde639..cb8d3e722f76 100644
> --- a/drivers/of/selftest.c
> +++ b/drivers/of/selftest.c
> @@ -30,6 +30,67 @@ static struct selftest_results {
> 	} \
> }
> 
> +static void __init of_selftest_dynamic(void)
> +{
> +	struct device_node *np;
> +	struct property *prop;
> +
> +	np = of_find_node_by_path("/testcase-data");
> +	if (!np) {
> +		pr_err("missing testcase data\n");
> +		return;
> +	}
> +
> +	/* Array of 4 properties for the purpose of testing */
> +	prop = kzalloc(sizeof(*prop) * 4, GFP_KERNEL);
> +	if (!prop) {
> +		selftest(0, "kzalloc() failed\n");
> +		return;
> +	}
> +
> +	/* Add a new property - should pass*/
> +	prop->name = "new-property";
> +	prop->value = "new-property-data";
> +	prop->length = strlen(prop->value);
> +	selftest(of_add_property(np, prop) == 0, "Adding a new property failed\n");
> +
> +	/* Try to add an existing property - should fail */
> +	prop++;
> +	prop->name = "new-property";
> +	prop->value = "new-property-data-should-fail";
> +	prop->length = strlen(prop->value);
> +	selftest(of_add_property(np, prop) != 0,
> +		 "Adding an existing property should have failed\n");
> +
> +	/* Try to modify an existing property - should pass */
> +	prop->value = "modify-property-data-should-pass";
> +	prop->length = strlen(prop->value);
> +	selftest(of_update_property(np, prop) == 0,
> +		 "Updating an existing property should have passed\n");
> +
> +	/* Try to modify non-existent property - should pass*/
> +	prop++;
> +	prop->name = "modify-property";
> +	prop->value = "modify-missing-property-data-should-pass";
> +	prop->length = strlen(prop->value);
> +	selftest(of_update_property(np, prop) == 0,
> +		 "Updating a missing property should have passed\n");
> +
> +	/* Remove property - should pass */
> +	selftest(of_remove_property(np, prop) == 0,
> +		 "Removing a property should have passed\n");
> +
> +	/* Adding very large property - should pass */
> +	prop++;
> +	prop->name = "large-property-PAGE_SIZEx8";
> +	prop->length = PAGE_SIZE * 8;
> +	prop->value = kzalloc(prop->length, GFP_KERNEL);
> +	selftest(prop->value != NULL, "Unable to allocate large buffer\n");
> +	if (prop->value)
> +		selftest(of_add_property(np, prop) == 0,
> +			 "Adding a large property should have passed\n");
> +}
> +
> static void __init of_selftest_parse_phandle_with_args(void)
> {
> 	struct device_node *np;
> @@ -312,6 +373,7 @@ static int __init of_selftest(void)
> 	of_node_put(np);
> 
> 	pr_info("start of selftest - you will see error messages\n");
> +	of_selftest_dynamic();
> 	of_selftest_parse_phandle_with_args();
> 	of_selftest_property_match_string();
> 	of_selftest_parse_interrupts();
> -- 
> 1.8.3.2
> 

I like this. I'll add my overlay test cases somewhere around here.

I might need to introduce some new helper functions in order to generate
the overlay completely programmatically.

Regards

-- Pantelis




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

* Re: [PATCH v5 2/3] of/selftest: Add self tests for manipulation of properties
  2013-11-15 17:46 ` [PATCH v5 2/3] of/selftest: Add self tests for manipulation of properties Grant Likely
  2013-11-16 17:50   ` Pantelis Antoniou
@ 2013-11-16 19:09   ` Rob Herring
  2013-11-17 22:13     ` Grant Likely
  1 sibling, 1 reply; 8+ messages in thread
From: Rob Herring @ 2013-11-16 19:09 UTC (permalink / raw)
  To: Grant Likely
  Cc: devicetree, linux-kernel, Rob Herring, Benjamin Herrenschmidt,
	David S. Miller, Nathan Fontenot, Pantelis Antoniou

On Fri, Nov 15, 2013 at 11:46 AM, Grant Likely <grant.likely@linaro.org> wrote:
> Adds a few simple test cases to ensure that addition, update and removal
> of device tree node properties works correctly.
>
> Signed-off-by: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  drivers/of/selftest.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)

Does this need to depend on or select OF_DYNAMIC?

Rob

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

* Re: [PATCH v5 2/3] of/selftest: Add self tests for manipulation of properties
  2013-11-16 19:09   ` Rob Herring
@ 2013-11-17 22:13     ` Grant Likely
  0 siblings, 0 replies; 8+ messages in thread
From: Grant Likely @ 2013-11-17 22:13 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, linux-kernel, Rob Herring, Benjamin Herrenschmidt,
	David S. Miller, Nathan Fontenot, Pantelis Antoniou

On Sat, 16 Nov 2013 13:09:13 -0600, Rob Herring <robherring2@gmail.com> wrote:
> On Fri, Nov 15, 2013 at 11:46 AM, Grant Likely <grant.likely@linaro.org> wrote:
> > Adds a few simple test cases to ensure that addition, update and removal
> > of device tree node properties works correctly.
> >
> > Signed-off-by: Grant Likely <grant.likely@linaro.org>
> > Cc: Rob Herring <rob.herring@calxeda.com>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: David S. Miller <davem@davemloft.net>
> > Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> > Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> > ---
> >  drivers/of/selftest.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 62 insertions(+)
> 
> Does this need to depend on or select OF_DYNAMIC?

It shouldn't. The property manipulation does not depend on OF_DYNAMIC.
Only the node manipulation does. When I do more testing next week I'll
verify.

g.

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

* Re: [PATCH v5 2/3] of/selftest: Add self tests for manipulation of properties
  2013-11-16 17:50   ` Pantelis Antoniou
@ 2013-11-17 22:14     ` Grant Likely
  0 siblings, 0 replies; 8+ messages in thread
From: Grant Likely @ 2013-11-17 22:14 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: devicetree, linux-kernel, Rob Herring, Benjamin Herrenschmidt,
	David S. Miller, Nathan Fontenot

On Sat, 16 Nov 2013 19:50:37 +0200, Pantelis Antoniou <panto@antoniou-consulting.com> wrote:
> Hi Grant,
> 
> On Nov 15, 2013, at 7:46 PM, Grant Likely wrote:
> 
> > Adds a few simple test cases to ensure that addition, update and removal
> > of device tree node properties works correctly.
> > 
> > Signed-off-by: Grant Likely <grant.likely@linaro.org>
> > Cc: Rob Herring <rob.herring@calxeda.com>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: David S. Miller <davem@davemloft.net>
> > Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> > Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> > ---
> > drivers/of/selftest.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 62 insertions(+)
> > 
> > diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
> > index e21012bde639..cb8d3e722f76 100644
> > --- a/drivers/of/selftest.c
> > +++ b/drivers/of/selftest.c
> > @@ -30,6 +30,67 @@ static struct selftest_results {
> > 	} \
> > }
> > 
> > +static void __init of_selftest_dynamic(void)
> > +{
> > +	struct device_node *np;
> > +	struct property *prop;
> > +
> > +	np = of_find_node_by_path("/testcase-data");
> > +	if (!np) {
> > +		pr_err("missing testcase data\n");
> > +		return;
> > +	}
> > +
> > +	/* Array of 4 properties for the purpose of testing */
> > +	prop = kzalloc(sizeof(*prop) * 4, GFP_KERNEL);
> > +	if (!prop) {
> > +		selftest(0, "kzalloc() failed\n");
> > +		return;
> > +	}
> > +
> > +	/* Add a new property - should pass*/
> > +	prop->name = "new-property";
> > +	prop->value = "new-property-data";
> > +	prop->length = strlen(prop->value);
> > +	selftest(of_add_property(np, prop) == 0, "Adding a new property failed\n");
> > +
> > +	/* Try to add an existing property - should fail */
> > +	prop++;
> > +	prop->name = "new-property";
> > +	prop->value = "new-property-data-should-fail";
> > +	prop->length = strlen(prop->value);
> > +	selftest(of_add_property(np, prop) != 0,
> > +		 "Adding an existing property should have failed\n");
> > +
> > +	/* Try to modify an existing property - should pass */
> > +	prop->value = "modify-property-data-should-pass";
> > +	prop->length = strlen(prop->value);
> > +	selftest(of_update_property(np, prop) == 0,
> > +		 "Updating an existing property should have passed\n");
> > +
> > +	/* Try to modify non-existent property - should pass*/
> > +	prop++;
> > +	prop->name = "modify-property";
> > +	prop->value = "modify-missing-property-data-should-pass";
> > +	prop->length = strlen(prop->value);
> > +	selftest(of_update_property(np, prop) == 0,
> > +		 "Updating a missing property should have passed\n");
> > +
> > +	/* Remove property - should pass */
> > +	selftest(of_remove_property(np, prop) == 0,
> > +		 "Removing a property should have passed\n");
> > +
> > +	/* Adding very large property - should pass */
> > +	prop++;
> > +	prop->name = "large-property-PAGE_SIZEx8";
> > +	prop->length = PAGE_SIZE * 8;
> > +	prop->value = kzalloc(prop->length, GFP_KERNEL);
> > +	selftest(prop->value != NULL, "Unable to allocate large buffer\n");
> > +	if (prop->value)
> > +		selftest(of_add_property(np, prop) == 0,
> > +			 "Adding a large property should have passed\n");
> > +}
> > +
> > static void __init of_selftest_parse_phandle_with_args(void)
> > {
> > 	struct device_node *np;
> > @@ -312,6 +373,7 @@ static int __init of_selftest(void)
> > 	of_node_put(np);
> > 
> > 	pr_info("start of selftest - you will see error messages\n");
> > +	of_selftest_dynamic();
> > 	of_selftest_parse_phandle_with_args();
> > 	of_selftest_property_match_string();
> > 	of_selftest_parse_interrupts();
> > -- 
> > 1.8.3.2
> > 
> 
> I like this. I'll add my overlay test cases somewhere around here.
> 
> I might need to introduce some new helper functions in order to generate
> the overlay completely programmatically.

That would be good. I also really want to get the overlay code working
so I can use it for loading the testcase data on any running system.
:-)

g.


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

end of thread, other threads:[~2013-11-17 22:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-15 17:46 [PATCH v5 0/3] Kobjectify device tree structures Grant Likely
     [not found] ` < 1384537604-5032-3-git-send-email-grant.likely@linaro.org>
2013-11-15 17:46 ` [PATCH v5 1/3] of: Make device nodes kobjects so they show up in sysfs Grant Likely
2013-11-15 17:46 ` [PATCH v5 2/3] of/selftest: Add self tests for manipulation of properties Grant Likely
2013-11-16 17:50   ` Pantelis Antoniou
2013-11-17 22:14     ` Grant Likely
2013-11-16 19:09   ` Rob Herring
2013-11-17 22:13     ` Grant Likely
2013-11-15 17:46 ` [PATCH v5 3/3] of: remove /proc/device-tree Grant Likely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).