regulator: core: Add of_node_get to of_regulator_match
PATCH 1/3 regulator: core: Add of_node_get to of_regulator_match
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.
--- drivers/regulator/of_regulator.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
drivers/regulator/of_regulator.c +3 -2
@@ -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 ; }
Signature · 1 line
1.7.2.5
PATCH 2/3 regulator: core: Add helper to put of_nodes from matches
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.
--- drivers/regulator/of_regulator.c | 21 +++++++++++++++++++++ include/linux/regulator/of_regulator.h | 7 +++++++ 2 files changed, 28 insertions(+), 0 deletions(-)
drivers/regulator/of_regulator.c +21 -0
@@ -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);
include/linux/regulator/of_regulator.h +7 -0
@@ -20,6 +20,8 @@ extern struct regulator_init_dataextern 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 */
Signature · 1 line
1.7.2.5
PATCH 3/3 regulator: core: Get and put regulator of_node
In reply to Charles KeepaxCurrently 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.
--- drivers/regulator/core.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/regulator/core.c +2 -1
@@ -3447,7 +3447,7 @@ regulator_register(const struct regulator_desc *regulator_desc,/* register with sysfs */ rdev->dev.class = ®ulator_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 (®ulator_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 (®ulator_list_mutex); }
Signature · 1 line
1.7.2.5
Quoted 5 lines · Charles Keepax
Currently,
of_regulator_matchdoes not increment the reference count of theof_nodesit 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 anof_node_getwhen we copy eachof_nodepointer 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
Quoted 9 lines · Mark Brown
Quoted 5 lines · Charles Keepax
Currently,
of_regulator_matchdoes not increment the reference count of theof_nodesit 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 anof_node_getwhen we copy eachof_nodepointer 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