* [PATCH 0/2] of: Create sysfs view of device tree nodes @ 2013-03-20 14:51 Grant Likely 2013-03-20 14:51 ` [PATCH 1/2] of: Make device nodes kobjects so they show up in sysfs Grant Likely 2013-03-20 14:51 ` [PATCH 2/2] of: remove /proc/device-tree Grant Likely 0 siblings, 2 replies; 20+ messages in thread From: Grant Likely @ 2013-03-20 14:51 UTC (permalink / raw) To: linux-kernel, devicetree-discuss Hi all, This series reworks the device tree userspace view to be exposed via sysfs. I've been wanting to move to using kobjects to manage the device tree for a while now. It results in less code overall, and it gives us the userspace view "for free". The first patch converts the device_nodes into kobjects and registers them under /sys/firmware/ofw/device-tree. The second removes the old /proc/device-tree support code and replaces it with a symlink from /proc/device-tree to the sysfs view. I attempted to also remove the virtual /proc/openprom filesystem, but unfortunately /proc/devicetree and /proc/openprom use different semantics. In /proc/devicetree the properties are exposed as raw binaries, but in /proc/openprom the properties are all converted into ascii string representation first. I've tested this series on ARM and embedded powerpc. I have build tested on SPARC and x86. Dave, I'm particularly interested to know how well this runs on SPARC. I think I've got it right, but I haven't tested the changes to the driver/of/pdt.c code. g. Documentation/ABI/testing/sysfs-firmware-ofw | 28 ++++++ drivers/of/Kconfig | 10 +- drivers/of/base.c | 180 +++++++++++++++++++++++------------- drivers/of/fdt.c | 3 +- drivers/of/pdt.c | 4 +- fs/proc/Makefile | 1 - fs/proc/proc_devtree.c | 243 ------------------------------------------------- fs/proc/root.c | 3 - include/linux/of.h | 10 +- include/linux/proc_fs.h | 16 ---- 10 files changed, 156 insertions(+), 342 deletions(-) [PATCH 1/2] of: Make device nodes kobjects so they show up in sysfs [PATCH 2/2] of: remove /proc/device-tree ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] of: Make device nodes kobjects so they show up in sysfs 2013-03-20 14:51 [PATCH 0/2] of: Create sysfs view of device tree nodes Grant Likely @ 2013-03-20 14:51 ` Grant Likely 2013-03-20 14:57 ` Benjamin Herrenschmidt 2013-03-20 14:51 ` [PATCH 2/2] of: remove /proc/device-tree Grant Likely 1 sibling, 1 reply; 20+ messages in thread From: Grant Likely @ 2013-03-20 14:51 UTC (permalink / raw) To: linux-kernel, devicetree-discuss Cc: Grant Likely, Rob Herring, Greg Kroah-Hartman, Benjamin Herrenschmidt, David S. Miller 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/device-tree Switching to sysfs does introduce two limitations however. First, normal sysfs attributes have a maximum size of PAGE_SIZE. Any properties larger than 4k will still show up in sysfs, but attempting to read them will result in truncated data. Practically speaking this should not be an issue assuming large binary blobs aren't encoded into the device tree. Second, all normal sysfs attributes report their size as 4096 bytes instead of the actual property size reported in /proc/device-tree. It is possible that this change will cause some userspace tools to break. Both of the above problems can be worked around by using sysfs_create_bin_file() instead of sysfs_create_file(), but doing so adds an additional 20 to 40 bytes of overhead per property. A device tree has a lot of properties in it. That overhead will very quickly add up. The other option is to create a new custom sysfs attribute type would solve the problem without additional overhead, but at the cost of more complexity in the sysfs support code. However, I'm hopeful that these problems are just imaginary and we can stick with normal sysfs attributes. Signed-off-by: Grant Likely <grant.likely@secretlab.ca> Cc: Rob Herring <rob.herring@calxeda.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: David S. Miller <davem@davemloft.net> --- Documentation/ABI/testing/sysfs-firmware-ofw | 28 ++++++ drivers/of/Kconfig | 2 +- drivers/of/base.c | 121 ++++++++++++++++++++++++-- drivers/of/fdt.c | 3 +- drivers/of/pdt.c | 4 +- include/linux/of.h | 9 +- 6 files changed, 154 insertions(+), 13 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 0000000..5ef9a77 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-firmware-ofw @@ -0,0 +1,28 @@ +What: /sys/firmware/ofw/../device-tree/ +Date: March 2013 +Contact: Grant Likely <grant.likely@secretlab.ca> +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/ofw. + + Userspace must not use the /sys/firmware/ofw/../device-tree + 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/ofw/../device-tree 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/drivers/of/Kconfig b/drivers/of/Kconfig index d37bfcf..d2d7be9 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -38,7 +38,7 @@ config OF_PROMTREE # Hardly any platforms need this. It is safe to select, but only do so if you # need it. config OF_DYNAMIC - bool + def_bool y config OF_ADDRESS def_bool y diff --git a/drivers/of/base.c b/drivers/of/base.c index 321d3ef..363e98b 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -22,6 +22,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" @@ -33,6 +34,12 @@ EXPORT_SYMBOL(of_allnodes); struct device_node *of_chosen; struct device_node *of_aliases; +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, @@ -83,14 +90,14 @@ EXPORT_SYMBOL(of_n_size_cells); 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); } /** @@ -100,16 +107,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; } @@ -142,11 +148,110 @@ 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); #endif /* CONFIG_OF_DYNAMIC */ +ssize_t of_node_sysfs_show(struct kobject *kobj, struct attribute *attr, char *buf) +{ + struct property *prop = container_of(attr, struct property, attr); + int length = prop->length; + + if (length >= PAGE_SIZE) + length = PAGE_SIZE-1; + memcpy(buf, prop->value, length); + return length; +} + +struct sysfs_ops of_node_sysfs_ops = { + .show = of_node_sysfs_show, +}; + +struct kobj_type of_node_ktype = { + .release = of_node_release, + .sysfs_ops = &of_node_sysfs_ops, +}; + +static bool of_init_complete = false; +static int __of_node_add(struct device_node *np) +{ + const char *name; + struct property *pp; + static int extra = 0; + int rc; + + np->kobj.kset = of_kset; + if (!np->parent) { + /* Nodes without parents are new top level trees */ + rc = kobject_add(&np->kobj, NULL, "device-tree-%i", extra++); + } else { + name = 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) { + /* + * Don't leak password data. Note: if this is ever switched to use + * sysfs_create_bin_file(), then it is very important to not expose + * the size of the password in the inode. If 'secure' is set, then + * the inode length must be reported as '0' + */ + bool secure = strncmp(pp->name, "security-", 9) == 0; + pp->attr.name = pp->name; + pp->attr.mode = secure ? S_IRUSR : S_IRUGO; + rc = sysfs_create_file(&np->kobj, &pp->attr); + WARN(rc, "error creating device node attribute\n"); + } + + 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_init_complete) + 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_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("ofw", 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); + of_init_complete = true; + mutex_unlock(&of_aliases_mutex); + return 0; +} +core_initcall(of_init); + static struct property *__of_find_property(const struct device_node *np, const char *name, int *lenp) { @@ -1445,6 +1550,7 @@ int of_attach_node(struct device_node *np) of_allnodes = np; raw_spin_unlock_irqrestore(&devtree_lock, flags); + of_node_add(np); of_add_proc_dt_entry(np); return 0; } @@ -1526,6 +1632,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 808be06..8807059 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -232,7 +232,6 @@ static unsigned long 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 unsigned long 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 37b56fd..2d9d7e1 100644 --- a/drivers/of/pdt.c +++ b/drivers/of/pdt.c @@ -180,8 +180,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; @@ -216,6 +214,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); @@ -246,6 +245,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 a0f1292..2399c8f 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 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); @@ -165,6 +168,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.7.10.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] of: Make device nodes kobjects so they show up in sysfs 2013-03-20 14:51 ` [PATCH 1/2] of: Make device nodes kobjects so they show up in sysfs Grant Likely @ 2013-03-20 14:57 ` Benjamin Herrenschmidt 2013-03-20 16:56 ` Greg Kroah-Hartman 2013-03-20 21:28 ` Grant Likely 0 siblings, 2 replies; 20+ messages in thread From: Benjamin Herrenschmidt @ 2013-03-20 14:57 UTC (permalink / raw) To: Grant Likely Cc: linux-kernel, devicetree-discuss, Rob Herring, Greg Kroah-Hartman, David S. Miller On Wed, 2013-03-20 at 14:51 +0000, Grant Likely wrote: > 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/device-tree > > Switching to sysfs does introduce two limitations however. First, normal > sysfs attributes have a maximum size of PAGE_SIZE. Any properties larger > than 4k will still show up in sysfs, but attempting to read them will > result in truncated data. Practically speaking this should not be an > issue assuming large binary blobs aren't encoded into the device tree. Unfortunately they occasionally are... VPDs can be pretty big for example. > Second, all normal sysfs attributes report their size as 4096 bytes > instead of the actual property size reported in /proc/device-tree. It is > possible that this change will cause some userspace tools to break. This is btw a complete idiocy of sysfs and should/could be fixed. > Both of the above problems can be worked around by using > sysfs_create_bin_file() instead of sysfs_create_file(), but doing so > adds an additional 20 to 40 bytes of overhead per property. A device > tree has a lot of properties in it. That overhead will very quickly add > up. The other option is to create a new custom sysfs attribute type > would solve the problem without additional overhead, but at the cost of > more complexity in the sysfs support code. However, I'm hopeful that > these problems are just imaginary and we can stick with normal sysfs > attributes. Disagreed. It would break stuff, especially the incorrect file size. Cheers, Ben. > Signed-off-by: Grant Likely <grant.likely@secretlab.ca> > Cc: Rob Herring <rob.herring@calxeda.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: David S. Miller <davem@davemloft.net> > --- > Documentation/ABI/testing/sysfs-firmware-ofw | 28 ++++++ > drivers/of/Kconfig | 2 +- > drivers/of/base.c | 121 ++++++++++++++++++++++++-- > drivers/of/fdt.c | 3 +- > drivers/of/pdt.c | 4 +- > include/linux/of.h | 9 +- > 6 files changed, 154 insertions(+), 13 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 0000000..5ef9a77 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-firmware-ofw > @@ -0,0 +1,28 @@ > +What: /sys/firmware/ofw/../device-tree/ > +Date: March 2013 > +Contact: Grant Likely <grant.likely@secretlab.ca> > +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/ofw. > + > + Userspace must not use the /sys/firmware/ofw/../device-tree > + 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/ofw/../device-tree 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/drivers/of/Kconfig b/drivers/of/Kconfig > index d37bfcf..d2d7be9 100644 > --- a/drivers/of/Kconfig > +++ b/drivers/of/Kconfig > @@ -38,7 +38,7 @@ config OF_PROMTREE > # Hardly any platforms need this. It is safe to select, but only do so if you > # need it. > config OF_DYNAMIC > - bool > + def_bool y > > config OF_ADDRESS > def_bool y > diff --git a/drivers/of/base.c b/drivers/of/base.c > index 321d3ef..363e98b 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -22,6 +22,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" > @@ -33,6 +34,12 @@ EXPORT_SYMBOL(of_allnodes); > struct device_node *of_chosen; > struct device_node *of_aliases; > > +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, > @@ -83,14 +90,14 @@ EXPORT_SYMBOL(of_n_size_cells); > 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); > } > > /** > @@ -100,16 +107,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; > } > > @@ -142,11 +148,110 @@ 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); > #endif /* CONFIG_OF_DYNAMIC */ > > +ssize_t of_node_sysfs_show(struct kobject *kobj, struct attribute *attr, char *buf) > +{ > + struct property *prop = container_of(attr, struct property, attr); > + int length = prop->length; > + > + if (length >= PAGE_SIZE) > + length = PAGE_SIZE-1; > + memcpy(buf, prop->value, length); > + return length; > +} > + > +struct sysfs_ops of_node_sysfs_ops = { > + .show = of_node_sysfs_show, > +}; > + > +struct kobj_type of_node_ktype = { > + .release = of_node_release, > + .sysfs_ops = &of_node_sysfs_ops, > +}; > + > +static bool of_init_complete = false; > +static int __of_node_add(struct device_node *np) > +{ > + const char *name; > + struct property *pp; > + static int extra = 0; > + int rc; > + > + np->kobj.kset = of_kset; > + if (!np->parent) { > + /* Nodes without parents are new top level trees */ > + rc = kobject_add(&np->kobj, NULL, "device-tree-%i", extra++); > + } else { > + name = 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) { > + /* > + * Don't leak password data. Note: if this is ever switched to use > + * sysfs_create_bin_file(), then it is very important to not expose > + * the size of the password in the inode. If 'secure' is set, then > + * the inode length must be reported as '0' > + */ > + bool secure = strncmp(pp->name, "security-", 9) == 0; > + pp->attr.name = pp->name; > + pp->attr.mode = secure ? S_IRUSR : S_IRUGO; > + rc = sysfs_create_file(&np->kobj, &pp->attr); > + WARN(rc, "error creating device node attribute\n"); > + } > + > + 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_init_complete) > + 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_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("ofw", 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); > + of_init_complete = true; > + mutex_unlock(&of_aliases_mutex); > + return 0; > +} > +core_initcall(of_init); > + > static struct property *__of_find_property(const struct device_node *np, > const char *name, int *lenp) > { > @@ -1445,6 +1550,7 @@ int of_attach_node(struct device_node *np) > of_allnodes = np; > raw_spin_unlock_irqrestore(&devtree_lock, flags); > > + of_node_add(np); > of_add_proc_dt_entry(np); > return 0; > } > @@ -1526,6 +1632,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 808be06..8807059 100644 > --- a/drivers/of/fdt.c > +++ b/drivers/of/fdt.c > @@ -232,7 +232,6 @@ static unsigned long 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 unsigned long 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 37b56fd..2d9d7e1 100644 > --- a/drivers/of/pdt.c > +++ b/drivers/of/pdt.c > @@ -180,8 +180,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; > @@ -216,6 +214,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); > @@ -246,6 +245,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 a0f1292..2399c8f 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 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); > @@ -165,6 +168,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) \ ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] of: Make device nodes kobjects so they show up in sysfs 2013-03-20 14:57 ` Benjamin Herrenschmidt @ 2013-03-20 16:56 ` Greg Kroah-Hartman 2013-03-20 17:46 ` Benjamin Herrenschmidt 2013-03-20 21:28 ` Grant Likely 1 sibling, 1 reply; 20+ messages in thread From: Greg Kroah-Hartman @ 2013-03-20 16:56 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Grant Likely, linux-kernel, devicetree-discuss, Rob Herring, David S. Miller On Wed, Mar 20, 2013 at 03:57:12PM +0100, Benjamin Herrenschmidt wrote: > On Wed, 2013-03-20 at 14:51 +0000, Grant Likely wrote: > > 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/device-tree > > > > Switching to sysfs does introduce two limitations however. First, normal > > sysfs attributes have a maximum size of PAGE_SIZE. Any properties larger > > than 4k will still show up in sysfs, but attempting to read them will > > result in truncated data. Practically speaking this should not be an > > issue assuming large binary blobs aren't encoded into the device tree. > > Unfortunately they occasionally are... VPDs can be pretty big for > example. If the attributes are binary blobs, use the binary file capability of sysfs to properly handle them. > > Second, all normal sysfs attributes report their size as 4096 bytes > > instead of the actual property size reported in /proc/device-tree. It is > > possible that this change will cause some userspace tools to break. > > This is btw a complete idiocy of sysfs and should/could be fixed. How can sysfs change this? It doesn't know the size of the attribute ahead of time, and it can change depending on what happens in the system. So we default to a page size, which is the largest size an attribute can be. thanks, greg k-h ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] of: Make device nodes kobjects so they show up in sysfs 2013-03-20 16:56 ` Greg Kroah-Hartman @ 2013-03-20 17:46 ` Benjamin Herrenschmidt 0 siblings, 0 replies; 20+ messages in thread From: Benjamin Herrenschmidt @ 2013-03-20 17:46 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Grant Likely, linux-kernel, devicetree-discuss, Rob Herring, David S. Miller On Wed, 2013-03-20 at 09:56 -0700, Greg Kroah-Hartman wrote: > > Unfortunately they occasionally are... VPDs can be pretty big for > > example. > > If the attributes are binary blobs, use the binary file capability of > sysfs to properly handle them. Except that we don't know that ... we have properties which comes all as "blobs", only the consumers can interpret what they contain. Tools like lsprop or dtc do have some built-in smarts to differentiate for example strings, simple numbers and blobs based roughly on heuristics of content & size but that's purely for the sake of pretty printing. Something like /proc/device-tree (or a sysfs equivalent) has no way really to know what's in there. So basically they should *all* be binary blobs. > > > Second, all normal sysfs attributes report their size as 4096 bytes > > > instead of the actual property size reported in /proc/device-tree. It is > > > possible that this change will cause some userspace tools to break. > > > > This is btw a complete idiocy of sysfs and should/could be fixed. > > How can sysfs change this? It doesn't know the size of the attribute > ahead of time, and it can change depending on what happens in the > system. So we default to a page size, which is the largest size an > attribute can be. That's true for some attributes I suppose and I while I do understand the difficulty there would be in calculating all the sizes on every stat, it's still gross, especially for those plenty of attributes who do have a fixed size. In our case, we do know the size, we should expose it. Cheers, Ben. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] of: Make device nodes kobjects so they show up in sysfs 2013-03-20 14:57 ` Benjamin Herrenschmidt 2013-03-20 16:56 ` Greg Kroah-Hartman @ 2013-03-20 21:28 ` Grant Likely 1 sibling, 0 replies; 20+ messages in thread From: Grant Likely @ 2013-03-20 21:28 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Linux Kernel Mailing List, devicetree-discuss, Rob Herring, Greg Kroah-Hartman, David S. Miller On Wed, Mar 20, 2013 at 2:57 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Wed, 2013-03-20 at 14:51 +0000, Grant Likely wrote: >> Both of the above problems can be worked around by using >> sysfs_create_bin_file() instead of sysfs_create_file(), but doing so >> adds an additional 20 to 40 bytes of overhead per property. A device >> tree has a lot of properties in it. That overhead will very quickly add >> up. The other option is to create a new custom sysfs attribute type >> would solve the problem without additional overhead, but at the cost of >> more complexity in the sysfs support code. However, I'm hopeful that >> these problems are just imaginary and we can stick with normal sysfs >> attributes. > > Disagreed. It would break stuff, especially the incorrect file size. I retried with bin files. Turns out I should stop making assumptions and actually try things. It requires less code, is simpler and pretty much eliminates the issues listed above. :-) I'll post a new series with the fixes. g. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] of: remove /proc/device-tree 2013-03-20 14:51 [PATCH 0/2] of: Create sysfs view of device tree nodes Grant Likely 2013-03-20 14:51 ` [PATCH 1/2] of: Make device nodes kobjects so they show up in sysfs Grant Likely @ 2013-03-20 14:51 ` Grant Likely 2013-03-20 14:57 ` Benjamin Herrenschmidt 2013-03-20 15:19 ` Rob Herring 1 sibling, 2 replies; 20+ messages in thread From: Grant Likely @ 2013-03-20 14:51 UTC (permalink / raw) To: linux-kernel, devicetree-discuss Cc: Grant Likely, Rob Herring, Greg Kroah-Hartman, Benjamin Herrenschmidt, David S. Miller 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. Signed-off-by: Grant Likely <grant.likely@secretlab.ca> Cc: Rob Herring <rob.herring@calxeda.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: David S. Miller <davem@davemloft.net> --- drivers/of/Kconfig | 8 -- drivers/of/base.c | 59 +----------- fs/proc/Makefile | 1 - fs/proc/proc_devtree.c | 243 ----------------------------------------------- fs/proc/root.c | 3 - include/linux/of.h | 1 - include/linux/proc_fs.h | 16 ---- 7 files changed, 2 insertions(+), 329 deletions(-) delete mode 100644 fs/proc/proc_devtree.c diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index d2d7be9..edb97e2 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" help diff --git a/drivers/of/base.c b/drivers/of/base.c index 363e98b..6714114 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -184,6 +184,8 @@ static int __of_node_add(struct device_node *np) np->kobj.kset = of_kset; if (!np->parent) { /* Nodes without parents are new top level trees */ + if (extra == 0) + proc_symlink("device-tree", NULL, "/sys/firmware/ofw/device-tree-0"); rc = kobject_add(&np->kobj, NULL, "device-tree-%i", extra++); } else { name = kbasename(np->full_name); @@ -1375,12 +1377,6 @@ int of_add_property(struct device_node *np, struct property *prop) *next = prop; raw_spin_unlock_irqrestore(&devtree_lock, flags); -#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; } @@ -1421,12 +1417,6 @@ int of_remove_property(struct device_node *np, struct property *prop) if (!found) return -ENODEV; -#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; } @@ -1475,12 +1465,6 @@ int of_update_property(struct device_node *np, struct property *newprop) if (!found) return -ENODEV; -#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; } @@ -1515,22 +1499,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. */ @@ -1551,31 +1519,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) -{ - struct device_node *parent = dn->parent; - struct property *prop = dn->properties; - - while (prop) { - remove_proc_entry(prop->name, dn->pde); - prop = prop->next; - } - - if (dn->pde) - remove_proc_entry(dn->pde->name, parent->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. * @@ -1631,7 +1577,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 712f24d..81d413b 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/proc_devtree.c b/fs/proc/proc_devtree.c deleted file mode 100644 index 30b590f..0000000 --- a/fs/proc/proc_devtree.c +++ /dev/null @@ -1,243 +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/module.h> -#include <linux/slab.h> -#include <asm/prom.h> -#include <asm/uaccess.h> -#include "internal.h" - -static inline void set_node_proc_entry(struct device_node *np, - struct proc_dir_entry *de) -{ -#ifdef HAVE_ARCH_DEVTREE_FIXUPS - np->pde = de; -#endif -} - -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(inode)->data); -} - -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 c6e9fac..04846a4 100644 --- a/fs/proc/root.c +++ b/fs/proc/root.c @@ -173,9 +173,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 2399c8f..f48302c 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; diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h index 8307f2f..90496b5 100644 --- a/include/linux/proc_fs.h +++ b/include/linux/proc_fs.h @@ -136,22 +136,6 @@ static inline void proc_tty_init(void) extern void proc_tty_register_driver(struct tty_driver *driver); extern void proc_tty_unregister_driver(struct tty_driver *driver); -/* - * proc_devtree.c - */ -#ifdef CONFIG_PROC_DEVICETREE -struct device_node; -struct property; -extern void proc_device_tree_init(void); -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 /* CONFIG_PROC_DEVICETREE */ - extern struct proc_dir_entry *proc_symlink(const char *, struct proc_dir_entry *, const char *); extern struct proc_dir_entry *proc_mkdir(const char *,struct proc_dir_entry *); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] of: remove /proc/device-tree 2013-03-20 14:51 ` [PATCH 2/2] of: remove /proc/device-tree Grant Likely @ 2013-03-20 14:57 ` Benjamin Herrenschmidt 2013-03-20 21:38 ` Grant Likely 2013-03-20 15:19 ` Rob Herring 1 sibling, 1 reply; 20+ messages in thread From: Benjamin Herrenschmidt @ 2013-03-20 14:57 UTC (permalink / raw) To: Grant Likely Cc: linux-kernel, devicetree-discuss, Rob Herring, Greg Kroah-Hartman, David S. Miller On Wed, 2013-03-20 at 14:51 +0000, Grant Likely wrote: > 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. NAK. It should at the very least be a CONFIG option for a while before completely switching over. Cheers, Ben. > Signed-off-by: Grant Likely <grant.likely@secretlab.ca> > Cc: Rob Herring <rob.herring@calxeda.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: David S. Miller <davem@davemloft.net> > --- > drivers/of/Kconfig | 8 -- > drivers/of/base.c | 59 +----------- > fs/proc/Makefile | 1 - > fs/proc/proc_devtree.c | 243 ----------------------------------------------- > fs/proc/root.c | 3 - > include/linux/of.h | 1 - > include/linux/proc_fs.h | 16 ---- > 7 files changed, 2 insertions(+), 329 deletions(-) > delete mode 100644 fs/proc/proc_devtree.c > > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig > index d2d7be9..edb97e2 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" > help > diff --git a/drivers/of/base.c b/drivers/of/base.c > index 363e98b..6714114 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -184,6 +184,8 @@ static int __of_node_add(struct device_node *np) > np->kobj.kset = of_kset; > if (!np->parent) { > /* Nodes without parents are new top level trees */ > + if (extra == 0) > + proc_symlink("device-tree", NULL, "/sys/firmware/ofw/device-tree-0"); > rc = kobject_add(&np->kobj, NULL, "device-tree-%i", extra++); > } else { > name = kbasename(np->full_name); > @@ -1375,12 +1377,6 @@ int of_add_property(struct device_node *np, struct property *prop) > *next = prop; > raw_spin_unlock_irqrestore(&devtree_lock, flags); > > -#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; > } > > @@ -1421,12 +1417,6 @@ int of_remove_property(struct device_node *np, struct property *prop) > if (!found) > return -ENODEV; > > -#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; > } > > @@ -1475,12 +1465,6 @@ int of_update_property(struct device_node *np, struct property *newprop) > if (!found) > return -ENODEV; > > -#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; > } > > @@ -1515,22 +1499,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. > */ > @@ -1551,31 +1519,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) > -{ > - struct device_node *parent = dn->parent; > - struct property *prop = dn->properties; > - > - while (prop) { > - remove_proc_entry(prop->name, dn->pde); > - prop = prop->next; > - } > - > - if (dn->pde) > - remove_proc_entry(dn->pde->name, parent->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. > * > @@ -1631,7 +1577,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 712f24d..81d413b 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/proc_devtree.c b/fs/proc/proc_devtree.c > deleted file mode 100644 > index 30b590f..0000000 > --- a/fs/proc/proc_devtree.c > +++ /dev/null > @@ -1,243 +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/module.h> > -#include <linux/slab.h> > -#include <asm/prom.h> > -#include <asm/uaccess.h> > -#include "internal.h" > - > -static inline void set_node_proc_entry(struct device_node *np, > - struct proc_dir_entry *de) > -{ > -#ifdef HAVE_ARCH_DEVTREE_FIXUPS > - np->pde = de; > -#endif > -} > - > -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(inode)->data); > -} > - > -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 c6e9fac..04846a4 100644 > --- a/fs/proc/root.c > +++ b/fs/proc/root.c > @@ -173,9 +173,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 2399c8f..f48302c 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; > diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h > index 8307f2f..90496b5 100644 > --- a/include/linux/proc_fs.h > +++ b/include/linux/proc_fs.h > @@ -136,22 +136,6 @@ static inline void proc_tty_init(void) > extern void proc_tty_register_driver(struct tty_driver *driver); > extern void proc_tty_unregister_driver(struct tty_driver *driver); > > -/* > - * proc_devtree.c > - */ > -#ifdef CONFIG_PROC_DEVICETREE > -struct device_node; > -struct property; > -extern void proc_device_tree_init(void); > -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 /* CONFIG_PROC_DEVICETREE */ > - > extern struct proc_dir_entry *proc_symlink(const char *, > struct proc_dir_entry *, const char *); > extern struct proc_dir_entry *proc_mkdir(const char *,struct proc_dir_entry *); ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] of: remove /proc/device-tree 2013-03-20 14:57 ` Benjamin Herrenschmidt @ 2013-03-20 21:38 ` Grant Likely 2013-03-21 4:19 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 20+ messages in thread From: Grant Likely @ 2013-03-20 21:38 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Linux Kernel Mailing List, devicetree-discuss, Rob Herring, Greg Kroah-Hartman, David S. Miller On Wed, Mar 20, 2013 at 2:57 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Wed, 2013-03-20 at 14:51 +0000, Grant Likely wrote: >> 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. > > NAK. It should at the very least be a CONFIG option for a while before > completely switching over. I'll modify patch 1 to create the symlink if CONFIG_PROC_DEVICETREE is not set. After the first patch can be applied we can leave it for a release or two before applying the second. g. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] of: remove /proc/device-tree 2013-03-20 21:38 ` Grant Likely @ 2013-03-21 4:19 ` Benjamin Herrenschmidt 2013-03-21 7:35 ` Grant Likely 0 siblings, 1 reply; 20+ messages in thread From: Benjamin Herrenschmidt @ 2013-03-21 4:19 UTC (permalink / raw) To: Grant Likely Cc: Linux Kernel Mailing List, devicetree-discuss, Rob Herring, Greg Kroah-Hartman, David S. Miller On Wed, 2013-03-20 at 21:38 +0000, Grant Likely wrote: > > NAK. It should at the very least be a CONFIG option for a while > before > > completely switching over. > > I'll modify patch 1 to create the symlink if CONFIG_PROC_DEVICETREE is > not set. After the first patch can be applied we can leave it for a > release or two before applying the second. Shouldn't we have the symlink just be a config option itself ? Eventually distros might want get rid of it completely .. Ben. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] of: remove /proc/device-tree 2013-03-21 4:19 ` Benjamin Herrenschmidt @ 2013-03-21 7:35 ` Grant Likely 2013-03-21 7:43 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 20+ messages in thread From: Grant Likely @ 2013-03-21 7:35 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Linux Kernel Mailing List, devicetree-discuss, Rob Herring, Greg Kroah-Hartman, David S. Miller On Thu, Mar 21, 2013 at 4:19 AM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Wed, 2013-03-20 at 21:38 +0000, Grant Likely wrote: >> > NAK. It should at the very least be a CONFIG option for a while >> before >> > completely switching over. >> >> I'll modify patch 1 to create the symlink if CONFIG_PROC_DEVICETREE is >> not set. After the first patch can be applied we can leave it for a >> release or two before applying the second. > > Shouldn't we have the symlink just be a config option itself ? > Eventually distros might want get rid of it completely .. Why? It is the cheapest thing in the world and it means the ABI doesn't change at all. g. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] of: remove /proc/device-tree 2013-03-21 7:35 ` Grant Likely @ 2013-03-21 7:43 ` Benjamin Herrenschmidt 2013-03-21 8:16 ` Grant Likely 0 siblings, 1 reply; 20+ messages in thread From: Benjamin Herrenschmidt @ 2013-03-21 7:43 UTC (permalink / raw) To: Grant Likely Cc: Linux Kernel Mailing List, devicetree-discuss, Rob Herring, Greg Kroah-Hartman, David S. Miller On Thu, 2013-03-21 at 07:35 +0000, Grant Likely wrote: > > Shouldn't we have the symlink just be a config option itself ? > > Eventually distros might want get rid of it completely .. > > Why? It is the cheapest thing in the world and it means the ABI > doesn't change at all. It's also gross and forces sysfs to remain in /sys which isn't a kernel enforced policy afaik. Cheers, Ben. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] of: remove /proc/device-tree 2013-03-21 7:43 ` Benjamin Herrenschmidt @ 2013-03-21 8:16 ` Grant Likely 2013-03-21 12:36 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 20+ messages in thread From: Grant Likely @ 2013-03-21 8:16 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Linux Kernel Mailing List, devicetree-discuss, Rob Herring, Greg Kroah-Hartman, David S. Miller On Thu, Mar 21, 2013 at 7:43 AM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Thu, 2013-03-21 at 07:35 +0000, Grant Likely wrote: >> > Shouldn't we have the symlink just be a config option itself ? >> > Eventually distros might want get rid of it completely .. >> >> Why? It is the cheapest thing in the world and it means the ABI >> doesn't change at all. > > It's also gross and forces sysfs to remain in /sys which isn't a kernel > enforced policy afaik. Documentation/sysfs-rules.txt, Line 30 g. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] of: remove /proc/device-tree 2013-03-21 8:16 ` Grant Likely @ 2013-03-21 12:36 ` Benjamin Herrenschmidt 2013-03-21 12:42 ` Grant Likely 2013-11-16 20:09 ` Geert Uytterhoeven 0 siblings, 2 replies; 20+ messages in thread From: Benjamin Herrenschmidt @ 2013-03-21 12:36 UTC (permalink / raw) To: Grant Likely Cc: Linux Kernel Mailing List, devicetree-discuss, Rob Herring, Greg Kroah-Hartman, David S. Miller On Thu, 2013-03-21 at 08:16 +0000, Grant Likely wrote: > On Thu, Mar 21, 2013 at 7:43 AM, Benjamin Herrenschmidt > <benh@kernel.crashing.org> wrote: > > On Thu, 2013-03-21 at 07:35 +0000, Grant Likely wrote: > >> > Shouldn't we have the symlink just be a config option itself ? > >> > Eventually distros might want get rid of it completely .. > >> > >> Why? It is the cheapest thing in the world and it means the ABI > >> doesn't change at all. > > > > It's also gross and forces sysfs to remain in /sys which isn't a kernel > > enforced policy afaik. > > Documentation/sysfs-rules.txt, Line 30 Whatever... it's still gross :-) Cheers, Ben. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] of: remove /proc/device-tree 2013-03-21 12:36 ` Benjamin Herrenschmidt @ 2013-03-21 12:42 ` Grant Likely 2013-11-16 20:09 ` Geert Uytterhoeven 1 sibling, 0 replies; 20+ messages in thread From: Grant Likely @ 2013-03-21 12:42 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Linux Kernel Mailing List, devicetree-discuss, Rob Herring, Greg Kroah-Hartman, David S. Miller On Thu, Mar 21, 2013 at 12:36 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Thu, 2013-03-21 at 08:16 +0000, Grant Likely wrote: >> On Thu, Mar 21, 2013 at 7:43 AM, Benjamin Herrenschmidt >> <benh@kernel.crashing.org> wrote: >> > On Thu, 2013-03-21 at 07:35 +0000, Grant Likely wrote: >> >> > Shouldn't we have the symlink just be a config option itself ? >> >> > Eventually distros might want get rid of it completely .. >> >> >> >> Why? It is the cheapest thing in the world and it means the ABI >> >> doesn't change at all. >> > >> > It's also gross and forces sysfs to remain in /sys which isn't a kernel >> > enforced policy afaik. >> >> Documentation/sysfs-rules.txt, Line 30 > > Whatever... it's still gross :-) ptffff! g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] of: remove /proc/device-tree 2013-03-21 12:36 ` Benjamin Herrenschmidt 2013-03-21 12:42 ` Grant Likely @ 2013-11-16 20:09 ` Geert Uytterhoeven 1 sibling, 0 replies; 20+ messages in thread From: Geert Uytterhoeven @ 2013-11-16 20:09 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Grant Likely, Linux Kernel Mailing List, devicetree-discuss, Rob Herring, Greg Kroah-Hartman, David S. Miller On Thu, Mar 21, 2013 at 1:36 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Thu, 2013-03-21 at 08:16 +0000, Grant Likely wrote: >> On Thu, Mar 21, 2013 at 7:43 AM, Benjamin Herrenschmidt >> <benh@kernel.crashing.org> wrote: >> > On Thu, 2013-03-21 at 07:35 +0000, Grant Likely wrote: >> >> > Shouldn't we have the symlink just be a config option itself ? >> >> > Eventually distros might want get rid of it completely .. >> >> >> >> Why? It is the cheapest thing in the world and it means the ABI >> >> doesn't change at all. >> > >> > It's also gross and forces sysfs to remain in /sys which isn't a kernel >> > enforced policy afaik. >> >> Documentation/sysfs-rules.txt, Line 30 > > Whatever... it's still gross :-) Even Android mounts it there ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] of: remove /proc/device-tree 2013-03-20 14:51 ` [PATCH 2/2] of: remove /proc/device-tree Grant Likely 2013-03-20 14:57 ` Benjamin Herrenschmidt @ 2013-03-20 15:19 ` Rob Herring 2013-03-20 16:24 ` Daniel Mack 1 sibling, 1 reply; 20+ messages in thread From: Rob Herring @ 2013-03-20 15:19 UTC (permalink / raw) To: Grant Likely Cc: linux-kernel, devicetree-discuss, Greg Kroah-Hartman, David S. Miller, Rob Herring On 03/20/2013 09:51 AM, Grant Likely wrote: > 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. I would suggest testing with lshw in particular. That's the only /proc/device-tree user I've come across. lshw can get system data from either DMI or DT, so it is a possible alternative to dmidecode on ARM. There aren't a large number of properties that it reads though. Rob > > Signed-off-by: Grant Likely <grant.likely@secretlab.ca> > Cc: Rob Herring <rob.herring@calxeda.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: David S. Miller <davem@davemloft.net> > --- > drivers/of/Kconfig | 8 -- > drivers/of/base.c | 59 +----------- > fs/proc/Makefile | 1 - > fs/proc/proc_devtree.c | 243 ----------------------------------------------- > fs/proc/root.c | 3 - > include/linux/of.h | 1 - > include/linux/proc_fs.h | 16 ---- > 7 files changed, 2 insertions(+), 329 deletions(-) > delete mode 100644 fs/proc/proc_devtree.c > > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig > index d2d7be9..edb97e2 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" > help > diff --git a/drivers/of/base.c b/drivers/of/base.c > index 363e98b..6714114 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -184,6 +184,8 @@ static int __of_node_add(struct device_node *np) > np->kobj.kset = of_kset; > if (!np->parent) { > /* Nodes without parents are new top level trees */ > + if (extra == 0) > + proc_symlink("device-tree", NULL, "/sys/firmware/ofw/device-tree-0"); > rc = kobject_add(&np->kobj, NULL, "device-tree-%i", extra++); > } else { > name = kbasename(np->full_name); > @@ -1375,12 +1377,6 @@ int of_add_property(struct device_node *np, struct property *prop) > *next = prop; > raw_spin_unlock_irqrestore(&devtree_lock, flags); > > -#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; > } > > @@ -1421,12 +1417,6 @@ int of_remove_property(struct device_node *np, struct property *prop) > if (!found) > return -ENODEV; > > -#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; > } > > @@ -1475,12 +1465,6 @@ int of_update_property(struct device_node *np, struct property *newprop) > if (!found) > return -ENODEV; > > -#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; > } > > @@ -1515,22 +1499,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. > */ > @@ -1551,31 +1519,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) > -{ > - struct device_node *parent = dn->parent; > - struct property *prop = dn->properties; > - > - while (prop) { > - remove_proc_entry(prop->name, dn->pde); > - prop = prop->next; > - } > - > - if (dn->pde) > - remove_proc_entry(dn->pde->name, parent->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. > * > @@ -1631,7 +1577,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 712f24d..81d413b 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/proc_devtree.c b/fs/proc/proc_devtree.c > deleted file mode 100644 > index 30b590f..0000000 > --- a/fs/proc/proc_devtree.c > +++ /dev/null > @@ -1,243 +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/module.h> > -#include <linux/slab.h> > -#include <asm/prom.h> > -#include <asm/uaccess.h> > -#include "internal.h" > - > -static inline void set_node_proc_entry(struct device_node *np, > - struct proc_dir_entry *de) > -{ > -#ifdef HAVE_ARCH_DEVTREE_FIXUPS > - np->pde = de; > -#endif > -} > - > -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(inode)->data); > -} > - > -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 c6e9fac..04846a4 100644 > --- a/fs/proc/root.c > +++ b/fs/proc/root.c > @@ -173,9 +173,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 2399c8f..f48302c 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; > diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h > index 8307f2f..90496b5 100644 > --- a/include/linux/proc_fs.h > +++ b/include/linux/proc_fs.h > @@ -136,22 +136,6 @@ static inline void proc_tty_init(void) > extern void proc_tty_register_driver(struct tty_driver *driver); > extern void proc_tty_unregister_driver(struct tty_driver *driver); > > -/* > - * proc_devtree.c > - */ > -#ifdef CONFIG_PROC_DEVICETREE > -struct device_node; > -struct property; > -extern void proc_device_tree_init(void); > -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 /* CONFIG_PROC_DEVICETREE */ > - > extern struct proc_dir_entry *proc_symlink(const char *, > struct proc_dir_entry *, const char *); > extern struct proc_dir_entry *proc_mkdir(const char *,struct proc_dir_entry *); > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] of: remove /proc/device-tree 2013-03-20 15:19 ` Rob Herring @ 2013-03-20 16:24 ` Daniel Mack 2013-03-20 16:40 ` Grant Likely 2013-03-21 4:03 ` Rob Landley 0 siblings, 2 replies; 20+ messages in thread From: Daniel Mack @ 2013-03-20 16:24 UTC (permalink / raw) To: Rob Herring Cc: Grant Likely, Greg Kroah-Hartman, devicetree-discuss, linux-kernel, Rob Herring, David S. Miller On Wed, Mar 20, 2013 at 4:19 PM, Rob Herring <robherring2@gmail.com> wrote: > On 03/20/2013 09:51 AM, Grant Likely wrote: >> 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. > > I would suggest testing with lshw in particular. That's the only > /proc/device-tree user I've come across. kexec is another one. Not to mention various vendor scripts that aren't necessarily public. Don't such things also fall under the "we do not break userspace compatibility - ever" rule? Daniel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] of: remove /proc/device-tree 2013-03-20 16:24 ` Daniel Mack @ 2013-03-20 16:40 ` Grant Likely 2013-03-21 4:03 ` Rob Landley 1 sibling, 0 replies; 20+ messages in thread From: Grant Likely @ 2013-03-20 16:40 UTC (permalink / raw) To: Daniel Mack Cc: Rob Herring, Greg Kroah-Hartman, devicetree-discuss, Linux Kernel Mailing List, Rob Herring, David S. Miller On Wed, Mar 20, 2013 at 4:24 PM, Daniel Mack <daniel@zonque.org> wrote: > On Wed, Mar 20, 2013 at 4:19 PM, Rob Herring <robherring2@gmail.com> wrote: >> On 03/20/2013 09:51 AM, Grant Likely wrote: >>> 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. >> >> I would suggest testing with lshw in particular. That's the only >> /proc/device-tree user I've come across. > > kexec is another one. Not to mention various vendor scripts that aren't > necessarily public. > > Don't such things also fall under the "we do not break userspace > compatibility - ever" rule? Correct. I've got no intention of applying this without testing the major users first. g. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] of: remove /proc/device-tree 2013-03-20 16:24 ` Daniel Mack 2013-03-20 16:40 ` Grant Likely @ 2013-03-21 4:03 ` Rob Landley 1 sibling, 0 replies; 20+ messages in thread From: Rob Landley @ 2013-03-21 4:03 UTC (permalink / raw) To: Daniel Mack Cc: Rob Herring, Grant Likely, Greg Kroah-Hartman, devicetree-discuss, linux-kernel, Rob Herring, David S. Miller On 03/20/2013 11:24:54 AM, Daniel Mack wrote: > On Wed, Mar 20, 2013 at 4:19 PM, Rob Herring <robherring2@gmail.com> > wrote: > > On 03/20/2013 09:51 AM, Grant Likely wrote: > >> 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. > > > > I would suggest testing with lshw in particular. That's the only > > /proc/device-tree user I've come across. > > kexec is another one. Not to mention various vendor scripts that > aren't > necessarily public. > > Don't such things also fall under the "we do not break userspace > compatibility - ever" rule? We used to have feature-removal-schedule. Linus removed it. (He did not add it to feature-removal-schedule first.) Rob ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2013-11-16 20:09 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-03-20 14:51 [PATCH 0/2] of: Create sysfs view of device tree nodes Grant Likely 2013-03-20 14:51 ` [PATCH 1/2] of: Make device nodes kobjects so they show up in sysfs Grant Likely 2013-03-20 14:57 ` Benjamin Herrenschmidt 2013-03-20 16:56 ` Greg Kroah-Hartman 2013-03-20 17:46 ` Benjamin Herrenschmidt 2013-03-20 21:28 ` Grant Likely 2013-03-20 14:51 ` [PATCH 2/2] of: remove /proc/device-tree Grant Likely 2013-03-20 14:57 ` Benjamin Herrenschmidt 2013-03-20 21:38 ` Grant Likely 2013-03-21 4:19 ` Benjamin Herrenschmidt 2013-03-21 7:35 ` Grant Likely 2013-03-21 7:43 ` Benjamin Herrenschmidt 2013-03-21 8:16 ` Grant Likely 2013-03-21 12:36 ` Benjamin Herrenschmidt 2013-03-21 12:42 ` Grant Likely 2013-11-16 20:09 ` Geert Uytterhoeven 2013-03-20 15:19 ` Rob Herring 2013-03-20 16:24 ` Daniel Mack 2013-03-20 16:40 ` Grant Likely 2013-03-21 4:03 ` Rob Landley
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).