[PATCH 1/3] regulator: core: Add of_node_get to of_regulator_match

[PATCH 1/3] regulator: core: Add of_node_get to of_regulator_match

[Charles Keepax]

Currently, of_regulator_match does not increment the reference count of
the of_nodes it takes new references to. This could cause the node
pointer held to be invalid, by the time it is passed to the regulator
core. This patchs adds an of_node_get when we copy each of_node pointer
into the match structure.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/regulator/of_regulator.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index ea4f36f..c58c8d1 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -119,7 +119,8 @@ EXPORT_SYMBOL_GPL(of_get_regulator_init_data);
  * regulator. The data parsed from a child node will be matched to a regulator
  * based on either the deprecated property regulator-compatible if present,
  * or otherwise the child node's name. Note that the match table is modified
- * in place.
+ * in place and an additional of_node reference is taken for each matched
+ * regulator.
  *
  * Returns the number of matches found or a negative error code on failure.
  */
@@ -162,7 +163,7 @@ int of_regulator_match(struct device *dev, struct device_node *node,
 					child->name);
 				return -EINVAL;
 			}
-			match->of_node = child;
+			match->of_node = of_node_get(child);
 			count++;
 			break;
 		}
-- 
1.7.2.5

[PATCH 2/3] regulator: core: Add helper to put of_nodes from matches

[Charles Keepax]

As of_regulator_match will take an of_node reference to each matched
regulator, it makes sense to provide a helper to put all those
references. This patch does that.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/regulator/of_regulator.c       |   21 +++++++++++++++++++++
 include/linux/regulator/of_regulator.h |    7 +++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index c58c8d1..188e0cb 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -172,3 +172,24 @@ int of_regulator_match(struct device *dev, struct device_node *node,
 	return count;
 }
 EXPORT_SYMBOL_GPL(of_regulator_match);
+
+/**
+ * of_regulator_put_match - put the of_node references from an
+ *                          of_regulator_match structure
+ * @matches: match table for the regulators
+ * @num_matches: number of entries in match table
+ *
+ * This function goes through a match table and calls of_node_put on each
+ * of_node.
+ */
+int of_regulator_put_match(struct of_regulator_match *matches,
+			   unsigned int num_matches)
+{
+	int i;
+
+	for (i = 0; i < num_matches; i++)
+		of_node_put(matches[i].of_node);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_regulator_put_match);
diff --git a/include/linux/regulator/of_regulator.h b/include/linux/regulator/of_regulator.h
index f921796..0652851 100644
--- a/include/linux/regulator/of_regulator.h
+++ b/include/linux/regulator/of_regulator.h
@@ -20,6 +20,8 @@ extern struct regulator_init_data
 extern int of_regulator_match(struct device *dev, struct device_node *node,
 			      struct of_regulator_match *matches,
 			      unsigned int num_matches);
+extern int of_regulator_put_match(struct of_regulator_match *matches,
+				  unsigned int num_matches);
 #else
 static inline struct regulator_init_data
 	*of_get_regulator_init_data(struct device *dev,
@@ -35,6 +37,11 @@ static inline int of_regulator_match(struct device *dev,
 {
 	return 0;
 }
+static inline int of_regulator_put_match(struct of_regulator_match *matches,
+					 unsigned int num_matches);
+{
+	return 0;
+}
 #endif /* CONFIG_OF */
 
 #endif /* __LINUX_OF_REG_H */
-- 
1.7.2.5

[PATCH 3/3] regulator: core: Get and put regulator of_node

[Charles Keepax]

Currently the regulator core does not take an additional reference to
the of_node it is passed. This means that the caller must ensure that
the of_node is valid for the duration of the regulator's existance.
It is reasonable for the framework to assume it is passed a valid
of_node but seems onerous for it to assume the caller will keep the node
valid for the life-time of the regulator, especially when
devm_regulator_register is used and there will likely be no code in the
driver called at the point it would be safe to put the of_node.

This patch adds an additional of_node_get when the regulator is
registered and an of_node_put when it is unregistered in the core. This
means individual drivers are free to put their of_node references at the
end of probe letting the regulator core handling it from there. This
simplifies code on the driver side.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/regulator/core.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index bac485a..5a673f6 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3447,7 +3447,7 @@ regulator_register(const struct regulator_desc *regulator_desc,
 
 	/* register with sysfs */
 	rdev->dev.class = &regulator_class;
-	rdev->dev.of_node = config->of_node;
+	rdev->dev.of_node = of_node_get(config->of_node);
 	rdev->dev.parent = dev;
 	dev_set_name(&rdev->dev, "regulator.%d",
 		     atomic_inc_return(&regulator_no) - 1);
@@ -3589,6 +3589,7 @@ void regulator_unregister(struct regulator_dev *rdev)
 	list_del(&rdev->list);
 	kfree(rdev->constraints);
 	regulator_ena_gpio_free(rdev);
+	of_node_put(rdev->dev.of_node);
 	device_unregister(&rdev->dev);
 	mutex_unlock(&regulator_list_mutex);
 }
-- 
1.7.2.5

Re: [PATCH 1/3] regulator: core: Add of_node_get to of_regulator_match

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 509 bytes --]

On Thu, Apr 03, 2014 at 03:32:15PM +0100, Charles Keepax wrote:
> Currently, of_regulator_match does not increment the reference count of
> the of_nodes it takes new references to. This could cause the node
> pointer held to be invalid, by the time it is passed to the regulator
> core. This patchs adds an of_node_get when we copy each of_node pointer
> into the match structure.

Applied all, thanks.  You just made all users of of_regulator_match()
buggy - I'm assuming you're going to be fixing that!  :P

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/3] regulator: core: Add of_node_get to of_regulator_match

[Charles Keepax]

On Thu, Apr 03, 2014 at 10:50:27PM +0100, Mark Brown wrote:
> On Thu, Apr 03, 2014 at 03:32:15PM +0100, Charles Keepax wrote:
> > Currently, of_regulator_match does not increment the reference count of
> > the of_nodes it takes new references to. This could cause the node
> > pointer held to be invalid, by the time it is passed to the regulator
> > core. This patchs adds an of_node_get when we copy each of_node pointer
> > into the match structure.
> 
> Applied all, thanks.  You just made all users of of_regulator_match()
> buggy - I'm assuming you're going to be fixing that!  :P

Yeah I am on it, just wanted to make sure this was an acceptable
approach before I went off doing loads of patches.

Thanks,
Charles