Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1579360 > unrolled thread

[PATCH v2] regulator: devres: introduce managed enable and disable operations

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] regulator: devres: introduce managed enable and disable  operations Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-02-13 03:40 +0100
    Re: [PATCH v2] regulator: devres: introduce managed enable and  disable operations Mark Brown <broonie@kernel.org> - 2017-02-13 19:10 +0100
      Re: [PATCH v2] regulator: devres: introduce managed enable and  disable operations Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-02-13 20:00 +0100

#1579360 — [PATCH v2] regulator: devres: introduce managed enable and disable operations

While preferred time to power up the device is when there are users of it
present (i.e. when device is "open"ed), there are times when it makes more
sense to power up the device in probe(). One such example is when device is
expected to be always used (such as a touchscreen on a mobile device, or
similar) and powering it in probe() simplifies driver code, when we need to
power up device to inspect it before completing probe, or when
re-initializing device is too slow/costly.

This patch introduces managed versions of regulator_enable() and
regulator_bulk_enable() so that such drivers do not mix managed and regular
resources, which is usually error-prone.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

v2: restored lost regulator_disable() stub

Mark, note that there is also patch introducing devm_clk_prepare() and
devm_clk_prepare_enable() that Russell did not hate so I think it will
get applied eventually. I believe lack of CLK methods was cited as a
reason for not having managed enable for regulators.

 drivers/regulator/devres.c         | 138 +++++++++++++++++++++++++++++++++++++
 include/linux/regulator/consumer.h |  34 +++++++++
 2 files changed, 172 insertions(+)

diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index 784e3bf32210..ada2081bc24e 100644
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -120,6 +120,62 @@ void devm_regulator_put(struct regulator *regulator)
 }
 EXPORT_SYMBOL_GPL(devm_regulator_put);
 
+static void __devm_regulator_disable(struct device *dev, void *res)
+{
+	regulator_disable(*(struct regulator **)res);
+}
+
+/**
+ * devm_regulator_enable - Resource managed regulator_enable()
+ * @regulator: regulator to enable
+ *
+ * Managed regulator_enable(). Regulators enabled by this function are
+ * automatically disabled on driver detach. See regulator_enable() for more
+ * information.
+ */
+int devm_regulator_enable(struct regulator *regulator)
+{
+	struct regulator **ptr;
+	int error;
+
+	ptr = devres_alloc(__devm_regulator_disable, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return -ENOMEM;
+
+	error = regulator_enable(regulator);
+	if (error) {
+		devres_free(ptr);
+		return error;
+	}
+
+	*ptr = regulator;
+	devres_add(regulator->dev, ptr);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_regulator_enable);
+
+/**
+ * devm_regulator_disable - Resource managed regulator_disable()
+ * @regulator: regulator to disable
+ *
+ * Disable a regulator enabled with devm_regulator_enable(). Normally
+ * this function will not need to be called and the resource management
+ * code will ensure that the regulator is disabled.
+ */
+int devm_regulator_disable(struct regulator *regulator)
+{
+	int error;
+
+	error = devres_destroy(regulator->dev, __devm_regulator_disable,
+			       devm_regulator_match, regulator);
+	if (WARN_ON(error))
+		return error;
+
+	return regulator_disable(regulator);
+}
+EXPORT_SYMBOL_GPL(devm_regulator_disable);
+
 struct regulator_bulk_devres {
 	struct regulator_bulk_data *consumers;
 	int num_consumers;
@@ -171,6 +227,88 @@ int devm_regulator_bulk_get(struct device *dev, int num_consumers,
 }
 EXPORT_SYMBOL_GPL(devm_regulator_bulk_get);
 
+static void __devm_regulator_bulk_disable(struct device *dev, void *res)
+{
+	struct regulator_bulk_devres *devres = res;
+
+	regulator_bulk_disable(devres->num_consumers, devres->consumers);
+}
+
+static int devm_regulator_bulk_match(struct device *dev, void *res, void *data)
+{
+	struct regulator_bulk_devres *r1 = res;
+	struct regulator_bulk_devres *r2 = data;
+
+	if (WARN_ON(!r1 || !r1->consumers || !r1->num_consumers))
+		return 0;
+
+	return r1->consumers == r2->consumers &&
+	       r1->num_consumers == r2->num_consumers;
+}
+
+/**
+ * devm_regulator_bulk_enable - Resource managed regulator_bulk_enable()
+ * @dev: device owning this resource
+ * @num_consumers: number of consumers in @consumers array
+ * @consumers: consumers that need be enabled
+ *
+ * Managed regulator_bulk_enable(). Regulators enabled by this function are
+ * automatically disabled on driver detach. See regulator_bulk_enable() for
+ * more information.
+ */
+int devm_regulator_bulk_enable(struct device *dev, int num_consumers,
+			       struct regulator_bulk_data *consumers)
+{
+	struct regulator_bulk_devres *devres;
+	int error;
+
+	devres = devres_alloc(__devm_regulator_bulk_disable,
+			      sizeof(*devres), GFP_KERNEL);
+	if (!devres)
+		return -ENOMEM;
+
+	error = regulator_bulk_enable(num_consumers, consumers);
+	if (error) {
+		devres_free(devres);
+		return error;
+	}
+
+	devres->consumers = consumers;
+	devres->num_consumers = num_consumers;
+	devres_add(dev, devres);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_regulator_bulk_enable);
+
+/**
+ * devm_regulator_bulk_disable - Resource managed regulator_bulk_disable()
+ * @dev: device owning this resource
+ * @num_consumers: number of consumers in @consumers array
+ * @consumers: consumers that were enabled with devm_regulator_bulk_enable()
+ *
+ * Disable regulators enabled with devm_regulator_bulk_enable(). Normally
+ * this function will not need to be called and the resource management
+ * code will ensure that the regulator is disabled.
+ */
+int devm_regulator_bulk_disable(struct device *dev, int num_consumers,
+				struct regulator_bulk_data *consumers)
+{
+	struct regulator_bulk_devres devres = {
+		.consumers	= consumers,
+		.num_consumers	= num_consumers,
+	};
+	int error;
+
+	error = devres_destroy(dev, __devm_regulator_disable,
+			       devm_regulator_bulk_match, &devres);
+	if (WARN_ON(error))
+		return error;
+
+	return regulator_bulk_disable(num_consumers, consumers);
+}
+EXPORT_SYMBOL_GPL(devm_regulator_bulk_disable);
+
 static void devm_rdev_release(struct device *dev, void *res)
 {
 	regulator_unregister(*(struct regulator_dev **)res);
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index acaeeec279af..0a81c9ac7b21 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -225,7 +225,9 @@ void devm_regulator_bulk_unregister_supply_alias(struct device *dev,
 
 /* regulator output control and status */
 int __must_check regulator_enable(struct regulator *regulator);
+int __must_check devm_regulator_enable(struct regulator *regulator);
 int regulator_disable(struct regulator *regulator);
+int devm_regulator_disable(struct regulator *regulator);
 int regulator_force_disable(struct regulator *regulator);
 int regulator_is_enabled(struct regulator *regulator);
 int regulator_disable_deferred(struct regulator *regulator, int ms);
@@ -236,8 +238,13 @@ int __must_check devm_regulator_bulk_get(struct device *dev, int num_consumers,
 					 struct regulator_bulk_data *consumers);
 int __must_check regulator_bulk_enable(int num_consumers,
 				       struct regulator_bulk_data *consumers);
+int __must_check devm_regulator_bulk_enable(struct device *dev,
+					int num_consumers,
+					struct regulator_bulk_data *consumers);
 int regulator_bulk_disable(int num_consumers,
 			   struct regulator_bulk_data *consumers);
+int devm_regulator_bulk_disable(struct device *dev, int num_consumers,
+				struct regulator_bulk_data *consumers);
 int regulator_bulk_force_disable(int num_consumers,
 			   struct regulator_bulk_data *consumers);
 void regulator_bulk_free(int num_consumers,
@@ -399,11 +406,23 @@ static inline int regulator_enable(struct regulator *regulator)
 	return 0;
 }
 
+static inline int devm_regulator_enable(struct device *dev,
+					struct regulator *regulator)
+{
+	return 0;
+}
+
 static inline int regulator_disable(struct regulator *regulator)
 {
 	return 0;
 }
 
+static inline int devm_regulator_disable(struct device *dev,
+					 struct regulator *regulator)
+{
+	return 0;
+}
+
 static inline int regulator_force_disable(struct regulator *regulator)
 {
 	return 0;
@@ -439,12 +458,27 @@ static inline int regulator_bulk_enable(int num_consumers,
 	return 0;
 }
 
+static inline int devm_regulator_bulk_enable(struct device *dev,
+					int num_consumers,
+					struct regulator_bulk_data *consumers)
+{
+	return 0;
+}
+
+
 static inline int regulator_bulk_disable(int num_consumers,
 					 struct regulator_bulk_data *consumers)
 {
 	return 0;
 }
 
+static inline int devm_regulator_bulk_disable(struct device *dev,
+					int num_consumers,
+					struct regulator_bulk_data *consumers)
+{
+	return 0;
+}
+
 static inline int regulator_bulk_force_disable(int num_consumers,
 					struct regulator_bulk_data *consumers)
 {
-- 
2.11.0.483.g087da7b7c-goog


-- 
Dmitry

[toc] | [next] | [standalone]

#1579973 — Re: [PATCH v2] regulator: devres: introduce managed enable and disable operations

[Multipart message — attachments visible in raw view] — view raw

On Sun, Feb 12, 2017 at 06:32:49PM -0800, Dmitry Torokhov wrote:

> v2: restored lost regulator_disable() stub

> Mark, note that there is also patch introducing devm_clk_prepare() and
> devm_clk_prepare_enable() that Russell did not hate so I think it will
> get applied eventually. I believe lack of CLK methods was cited as a
> reason for not having managed enable for regulators.

No, that's never been an issue.  The concern is partly that nobody
bothered writing the patch but also that it gets messy over suspend and
resume since you end up with drivers either doing explicit releases of
managed resources (which is not normally a good pattern) or mixing
managed and unmanaged access to the same resource which is also fun.  

[toc] | [prev] | [next] | [standalone]

#1579997 — Re: [PATCH v2] regulator: devres: introduce managed enable and disable operations

On Mon, Feb 13, 2017 at 06:01:29PM +0000, Mark Brown wrote:
> On Sun, Feb 12, 2017 at 06:32:49PM -0800, Dmitry Torokhov wrote:
> 
> > v2: restored lost regulator_disable() stub
> 
> > Mark, note that there is also patch introducing devm_clk_prepare() and
> > devm_clk_prepare_enable() that Russell did not hate so I think it will
> > get applied eventually. I believe lack of CLK methods was cited as a
> > reason for not having managed enable for regulators.
> 
> No, that's never been an issue.  The concern is partly that nobody
> bothered writing the patch but also that it gets messy over suspend and
> resume since you end up with drivers either doing explicit releases of
> managed resources (which is not normally a good pattern) or mixing
> managed and unmanaged access to the same resource which is also fun.  

I see where you are coming from, but I think that it is lesser concern
than mixing managed and unmanaged resources in probe() and remove() and
making sure that release order is right when they are mixed like that.

I think it is helps if you think about devm_regulator_enable and regular
regulator_enable as managed and unmanaged *actions*, not resources. So
managed action of enabling regulator will be undone on remove() and you
have to manually undo unmanaged regulator_disable() on resume(). It is
not worse than having unbalanced regulator_enable/disable between
probe()/suspend()/resume()/remove().

Thanks.

-- 
Dmitry

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web