[PATCH 2/3] of: Introduce safe accessors for node->data
Anton Vorontsov
associated with a node.
Previously there was no need for any locks and accessors since we were
initializing the data mostly at boot time and never modified it later.
Though, nowadays OF GPIO infrastructure supports GPIO chips detaching,
so to handle this correctly we have to introduce locking for the
node->data field.
Signed-off-by: Anton Vorontsov <avoro...@ru.mvista.com>
---
drivers/of/base.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/of.h | 5 +++++
2 files changed, 50 insertions(+), 0 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 716d439..41ed6ba 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -36,8 +36,53 @@ void of_node_init(struct device_node *np)
{
memset(np, 0, sizeof(*np));
kref_init(&np->kref);
+ spin_lock_init(&np->data_lock);
}
+/**
+ * of_node_set_data - Try to set node->data
+ * @node: Node
+ * @data: Data
+ *
+ * This function tries to safely set node->data. Returns 0 on success,
+ * -EBUSY if node->data was already set.
+ */
+int of_node_set_data(struct device_node *np, void *data)
+{
+ int ret = 0;
+ unsigned long flags;
+
+ spin_lock_irqsave(&np->data_lock, flags);
+ if (np->data)
+ ret = -EBUSY;
+ else
+ np->data = data;
+ spin_unlock_irqrestore(&np->data_lock, flags);
+
+ if (!ret)
+ of_node_get(np);
+
+ return ret;
+}
+EXPORT_SYMBOL(of_node_set_data);
+
+/**
+ * of_node_release_data_unlocked - Release node->data
+ * @node: Node
+ *
+ * This function releases node->data, so that others could set it.
+ *
+ * This function doesn't grab any locks, so that a caller can grab the
+ * lock itself, atomically read the data and decide if it wants to
+ * release it.
+ */
+void of_node_release_data_unlocked(struct device_node *np)
+{
+ np->data = NULL;
+ of_node_put(np);
+}
+EXPORT_SYMBOL(of_node_release_data_unlocked);
+
int of_n_addr_cells(struct device_node *np)
{
const int *ip;
diff --git a/include/linux/of.h b/include/linux/of.h
index 717d690..c573ee2 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -18,6 +18,7 @@
#include <linux/types.h>
#include <linux/bitops.h>
#include <linux/kref.h>
+#include <linux/spinlock.h>
#include <linux/mod_devicetable.h>
typedef u32 phandle;
@@ -56,6 +57,7 @@ struct device_node {
struct kref kref;
unsigned long _flags;
void *data;
+ spinlock_t data_lock;
#if defined(CONFIG_SPARC)
char *path_component_name;
unsigned int unique_id;
@@ -65,6 +67,9 @@ struct device_node {
extern void of_node_init(struct device_node *np);
+extern int of_node_set_data(struct device_node *np, void *data);
+extern void of_node_release_data_unlocked(struct device_node *np);
+
static inline int of_node_check_flag(struct device_node *n, unsigned long flag)
{
return test_bit(flag, &n->_flags);
--
1.6.5.7
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Grant Likely
<avoro...@ru.mvista.com> wrote:
> Platform code use node->data to store some private information
> associated with a node.
>
> Previously there was no need for any locks and accessors since we were
> initializing the data mostly at boot time and never modified it later.
>
> Though, nowadays OF GPIO infrastructure supports GPIO chips detaching,
> so to handle this correctly we have to introduce locking for the
> node->data field.
I'm not convinced this is needed. What's wrong with using the
whole-tree devtree_lock?
g.
Anton Vorontsov
> On Fri, Feb 5, 2010 at 1:50 PM, Anton Vorontsov
> <avoro...@ru.mvista.com> wrote:
> > Platform code use node->data to store some private information
> > associated with a node.
> >
> > Previously there was no need for any locks and accessors since we were
> > initializing the data mostly at boot time and never modified it later.
> >
> > Though, nowadays OF GPIO infrastructure supports GPIO chips detaching,
> > so to handle this correctly we have to introduce locking for the
> > node->data field.
>
> I'm not convinced this is needed. What's wrong with using the
> whole-tree devtree_lock?
Why are you concerned? It doesn't add much of any footprint.
$ grep -c { -r arch/powerpc/boot/dts/ | cut -d: -f2 | sort -n | tail -n1
84
So far we have max 84 nodes, so it's a few hundreds of bytes for all
the dev tree.
Anyway, yes, we can use the devtree lock. Though, this will require a
bit more modifications, and I'm not sure if it's a great idea in
general (i.e. using the global lock in contrast to fine grained
locking).
The thing is that you can't use most of the of_ functions when you
hold the devtree lock (IIRC, rwlock has the same restrictions as a
spinlock, so you can't nest these locks).
I can try to rework OF GPIO calls so that they won't require of_
calls when they hold the lock, and let's see how it'll look like.
Thanks!
--
Anton Vorontsov
email: cbouat...@gmail.com
irc://irc.freenode.net/bd2