[PATCH 1/3] regulator: core: Add devres versions of notifier registration

[PATCH 1/3] regulator: core: Add devres versions of notifier registration

[Charles Keepax]

From: Charles Keepax <ckeepax@gmail.com>

Add devm_regulator_register_notifier, this adds the resource against the
device for the consumer supply we are registering the notifier for. There
seem to be few use-cases where this wouldn't be the users intention and
this ensures the notifiers will always be removed at the correct time.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---

I have based this series of your ASoC tree Mark, because the
wm8804 patches have dependencies there. But just let me know
if you would rather I send the regulator patch against your
regulator tree.

Thanks,
Charles

 drivers/regulator/devres.c         |   85 ++++++++++++++++++++++++++++++++++++
 include/linux/regulator/consumer.h |   16 +++++++
 2 files changed, 101 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index 8f785bc..6ec1d40 100644
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -413,3 +413,88 @@ void devm_regulator_bulk_unregister_supply_alias(struct device *dev,
 		devm_regulator_unregister_supply_alias(dev, id[i]);
 }
 EXPORT_SYMBOL_GPL(devm_regulator_bulk_unregister_supply_alias);
+
+struct regulator_notifier_match {
+	struct regulator *regulator;
+	struct notifier_block *nb;
+};
+
+static int devm_regulator_match_notifier(struct device *dev, void *res,
+					 void *data)
+{
+	struct regulator_notifier_match *match = res;
+	struct regulator_notifier_match *target = data;
+
+	return match->regulator == target->regulator && match->nb == target->nb;
+}
+
+static void devm_regulator_destroy_notifier(struct device *dev, void *res)
+{
+	struct regulator_notifier_match *match = res;
+
+	regulator_unregister_notifier(match->regulator, match->nb);
+}
+
+/**
+ * devm_regulator_register_notifier - Resource managed
+ * regulator_register_notifier
+ *
+ * @regulator: regulator source
+ * @nb: notifier block
+ *
+ * The notifier will be registers under the consumer device and be
+ * automatically be unregistered when the source device is unbound.
+ */
+int devm_regulator_register_notifier(struct regulator *regulator,
+				     struct notifier_block *nb)
+{
+	struct regulator_notifier_match *match;
+	int ret;
+
+	match = devres_alloc(devm_regulator_destroy_notifier,
+			     sizeof(struct regulator_notifier_match),
+			     GFP_KERNEL);
+	if (!match)
+		return -ENOMEM;
+
+	match->regulator = regulator;
+	match->nb = nb;
+
+	ret = regulator_register_notifier(regulator, nb);
+	if (ret < 0) {
+		devres_free(match);
+		return ret;
+	}
+
+	devres_add(regulator->dev, match);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_regulator_register_notifier);
+
+/**
+ * devm_regulator_unregister_notifier - Resource managed
+ * regulator_unregister_notifier()
+ *
+ * @regulator: regulator source
+ * @nb: notifier block
+ *
+ * Unregister a notifier registered with devm_regulator_register_notifier().
+ * Normally this function will not need to be called and the resource
+ * management code will ensure that the resource is freed.
+ */
+void devm_regulator_unregister_notifier(struct regulator *regulator,
+					struct notifier_block *nb)
+{
+	struct regulator_notifier_match match;
+	int rc;
+
+	match.regulator = regulator;
+	match.nb = nb;
+
+	rc = devres_release(regulator->dev, devm_regulator_destroy_notifier,
+			    devm_regulator_match_notifier, &match);
+	if (rc != 0)
+		WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_regulator_unregister_notifier);
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index d17e1ff..bd631ee 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -252,8 +252,12 @@ int regulator_list_hardware_vsel(struct regulator *regulator,
 /* regulator notifier block */
 int regulator_register_notifier(struct regulator *regulator,
 			      struct notifier_block *nb);
+int devm_regulator_register_notifier(struct regulator *regulator,
+				     struct notifier_block *nb);
 int regulator_unregister_notifier(struct regulator *regulator,
 				struct notifier_block *nb);
+void devm_regulator_unregister_notifier(struct regulator *regulator,
+					struct notifier_block *nb);
 
 /* driver data - core doesn't touch */
 void *regulator_get_drvdata(struct regulator *regulator);
@@ -515,12 +519,24 @@ static inline int regulator_register_notifier(struct regulator *regulator,
 	return 0;
 }
 
+static inline int devm_regulator_register_notifier(struct regulator *regulator,
+						   struct notifier_block *nb)
+{
+	return 0;
+}
+
 static inline int regulator_unregister_notifier(struct regulator *regulator,
 				struct notifier_block *nb)
 {
 	return 0;
 }
 
+static inline int devm_regulator_unregister_notifier(struct regulator *regulator,
+						     struct notifier_block *nb)
+{
+	return 0;
+}
+
 static inline void *regulator_get_drvdata(struct regulator *regulator)
 {
 	return NULL;
-- 
1.7.2.5

[PATCH 2/3] ASoC: wm8804: Use new devres regulator_register_notifier

[Charles Keepax]

From: Charles Keepax <ckeepax@gmail.com>

This is more idiomatic and also fixes an issue where the notifiers were
being leaked if probe failed.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm8804.c |   15 ++++-----------
 1 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/sound/soc/codecs/wm8804.c b/sound/soc/codecs/wm8804.c
index 1bd4ace..7804ddf 100644
--- a/sound/soc/codecs/wm8804.c
+++ b/sound/soc/codecs/wm8804.c
@@ -601,8 +601,10 @@ int wm8804_probe(struct device *dev, struct regmap *regmap)
 
 	/* This should really be moved into the regulator core */
 	for (i = 0; i < ARRAY_SIZE(wm8804->supplies); i++) {
-		ret = regulator_register_notifier(wm8804->supplies[i].consumer,
-						  &wm8804->disable_nb[i]);
+		struct regulator *regulator = wm8804->supplies[i].consumer;
+
+		ret = devm_regulator_register_notifier(regulator,
+						       &wm8804->disable_nb[i]);
 		if (ret != 0) {
 			dev_err(dev,
 				"Failed to register regulator notifier: %d\n",
@@ -662,15 +664,6 @@ EXPORT_SYMBOL_GPL(wm8804_probe);
 
 void wm8804_remove(struct device *dev)
 {
-	struct wm8804_priv *wm8804;
-	int i;
-
-	wm8804 = dev_get_drvdata(dev);
-
-	for (i = 0; i < ARRAY_SIZE(wm8804->supplies); ++i)
-		regulator_unregister_notifier(wm8804->supplies[i].consumer,
-					      &wm8804->disable_nb[i]);
-
 	snd_soc_unregister_codec(dev);
 }
 EXPORT_SYMBOL_GPL(wm8804_remove);
-- 
1.7.2.5

[PATCH 3/3] ASoC: wm8804: Fix small issues in probe error paths

[Charles Keepax]

This patch fixes some small issues on the probe error paths. Firstly,
fail probe if we can't register the regulator notifiers as this
will cause the cache to never be synchronised which will result in odd
behaviour if the regulators are controllable. Secondly, we don't need to
call regulator_bulk_disable if the enable fails, because the regulator
core will handle this clean up for us. Finally, we need to disable the
regulators if snd_soc_register_codecs fails.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm8804.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/wm8804.c b/sound/soc/codecs/wm8804.c
index 7804ddf..f44da83 100644
--- a/sound/soc/codecs/wm8804.c
+++ b/sound/soc/codecs/wm8804.c
@@ -609,6 +609,7 @@ int wm8804_probe(struct device *dev, struct regmap *regmap)
 			dev_err(dev,
 				"Failed to register regulator notifier: %d\n",
 				ret);
+			return ret;
 		}
 	}
 
@@ -616,7 +617,7 @@ int wm8804_probe(struct device *dev, struct regmap *regmap)
 				    wm8804->supplies);
 	if (ret) {
 		dev_err(dev, "Failed to enable supplies: %d\n", ret);
-		goto err_reg_enable;
+		return ret;
 	}
 
 	ret = regmap_read(regmap, WM8804_RST_DEVID1, &id1);
@@ -653,8 +654,14 @@ int wm8804_probe(struct device *dev, struct regmap *regmap)
 		goto err_reg_enable;
 	}
 
-	return snd_soc_register_codec(dev, &soc_codec_dev_wm8804,
-				      &wm8804_dai, 1);
+	ret = snd_soc_register_codec(dev, &soc_codec_dev_wm8804,
+				     &wm8804_dai, 1);
+	if (ret < 0) {
+		dev_err(dev, "Failed to register CODEC: %d\n", ret);
+		goto err_reg_enable;
+	}
+
+	return 0;
 
 err_reg_enable:
 	regulator_bulk_disable(ARRAY_SIZE(wm8804->supplies), wm8804->supplies);
-- 
1.7.2.5

[PATCH 3/3] ASoC: wm8804: Fix small issues in probe error paths

[Charles Keepax]

This patch fixes some small issues on the probe error paths. Firstly,
fail probe if we can't register the regulator notifiers as this
will cause the cache to never be synchronised which will result in odd
behaviour if the regulators are controllable. Secondly, we don't need to
call regulator_bulk_disable if the enable fails, because the regulator
core will handle this clean up for us. Finally, we need to disable the
regulators if snd_soc_register_codecs fails.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm8804.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/wm8804.c b/sound/soc/codecs/wm8804.c
index 7804ddf..f44da83 100644
--- a/sound/soc/codecs/wm8804.c
+++ b/sound/soc/codecs/wm8804.c
@@ -609,6 +609,7 @@ int wm8804_probe(struct device *dev, struct regmap *regmap)
 			dev_err(dev,
 				"Failed to register regulator notifier: %d\n",
 				ret);
+			return ret;
 		}
 	}
 
@@ -616,7 +617,7 @@ int wm8804_probe(struct device *dev, struct regmap *regmap)
 				    wm8804->supplies);
 	if (ret) {
 		dev_err(dev, "Failed to enable supplies: %d\n", ret);
-		goto err_reg_enable;
+		return ret;
 	}
 
 	ret = regmap_read(regmap, WM8804_RST_DEVID1, &id1);
@@ -653,8 +654,14 @@ int wm8804_probe(struct device *dev, struct regmap *regmap)
 		goto err_reg_enable;
 	}
 
-	return snd_soc_register_codec(dev, &soc_codec_dev_wm8804,
-				      &wm8804_dai, 1);
+	ret = snd_soc_register_codec(dev, &soc_codec_dev_wm8804,
+				     &wm8804_dai, 1);
+	if (ret < 0) {
+		dev_err(dev, "Failed to register CODEC: %d\n", ret);
+		goto err_reg_enable;
+	}
+
+	return 0;
 
 err_reg_enable:
 	regulator_bulk_disable(ARRAY_SIZE(wm8804->supplies), wm8804->supplies);
-- 
1.7.2.5

Re: [PATCH 1/3] regulator: core: Add devres versions of notifier registration

[Mark Brown]

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

On Thu, Mar 05, 2015 at 03:39:20PM +0000, Charles Keepax wrote:
> From: Charles Keepax <ckeepax@gmail.com>
> 
> Add devm_regulator_register_notifier, this adds the resource against the
> device for the consumer supply we are registering the notifier for. There
> seem to be few use-cases where this wouldn't be the users intention and
> this ensures the notifiers will always be removed at the correct time.

Applied all, thanks.

> I have based this series of your ASoC tree Mark, because the
> wm8804 patches have dependencies there. But just let me know
> if you would rather I send the regulator patch against your
> regulator tree.

You should send against the relevant tree for the subsystem usually, but
in this case they're the same as there's no relevant regulator changes.

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