[PATCH v2] regulator: Propagate voltage changes to supply regulators

[PATCH v2] regulator: Propagate voltage changes to supply regulators

[Sascha Hauer]

In-Reply-To: 

Until now changing the voltage of a regulator only ever effected the
regulator itself, but never its supplies. It's a common pattern though
to put LDO regulators behind switching regulators. The switching
regulators efficiently drop the input voltage but have a high ripple on
their output. The output is then cleaned up by the LDOs. For higher
energy efficiency the voltage drop at the LDOs should be minimized. This
series adds support for such a scenario. Another case voltage
propagation is useful is simple switches which are abstracted as
regulators. These can now offer voltage settings to their consumers
which are transparently passed to the switches supply.

Please review, any input welcome.

Changes since RFC (v1):
- split into more patches
- Only do voltage propagation when we have a supply and either a minimum
  dropout voltage is specified or the regulator is a switch (lacks a
  get_voltage operation)

Sascha

[PATCH 1/8] regulator: core: create unlocked version of regulator_list_voltage

[Sascha Hauer]

The unlocked version will be needed when we start propagating voltage
changes to the supply regulators.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/regulator/core.c | 62 +++++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 27 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 8a34f6a..bab426d 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2312,6 +2312,40 @@ static int _regulator_is_enabled(struct regulator_dev *rdev)
 	return rdev->desc->ops->is_enabled(rdev);
 }
 
+static int _regulator_list_voltage(struct regulator *regulator,
+				    unsigned selector, int lock)
+{
+	struct regulator_dev *rdev = regulator->rdev;
+	const struct regulator_ops *ops = rdev->desc->ops;
+	int ret;
+
+	if (rdev->desc->fixed_uV && rdev->desc->n_voltages == 1 && !selector)
+		return rdev->desc->fixed_uV;
+
+	if (ops->list_voltage) {
+		if (selector >= rdev->desc->n_voltages)
+			return -EINVAL;
+		if (lock)
+			mutex_lock(&rdev->mutex);
+		ret = ops->list_voltage(rdev, selector);
+		if (lock)
+			mutex_unlock(&rdev->mutex);
+	} else if (rdev->supply) {
+		ret = _regulator_list_voltage(rdev->supply, selector, lock);
+	} else {
+		return -EINVAL;
+	}
+
+	if (ret > 0) {
+		if (ret < rdev->constraints->min_uV)
+			ret = 0;
+		else if (ret > rdev->constraints->max_uV)
+			ret = 0;
+	}
+
+	return ret;
+}
+
 /**
  * regulator_is_enabled - is the regulator output enabled
  * @regulator: regulator source
@@ -2401,33 +2435,7 @@ EXPORT_SYMBOL_GPL(regulator_count_voltages);
  */
 int regulator_list_voltage(struct regulator *regulator, unsigned selector)
 {
-	struct regulator_dev *rdev = regulator->rdev;
-	const struct regulator_ops *ops = rdev->desc->ops;
-	int ret;
-
-	if (rdev->desc->fixed_uV && rdev->desc->n_voltages == 1 && !selector)
-		return rdev->desc->fixed_uV;
-
-	if (ops->list_voltage) {
-		if (selector >= rdev->desc->n_voltages)
-			return -EINVAL;
-		mutex_lock(&rdev->mutex);
-		ret = ops->list_voltage(rdev, selector);
-		mutex_unlock(&rdev->mutex);
-	} else if (rdev->supply) {
-		ret = regulator_list_voltage(rdev->supply, selector);
-	} else {
-		return -EINVAL;
-	}
-
-	if (ret > 0) {
-		if (ret < rdev->constraints->min_uV)
-			ret = 0;
-		else if (ret > rdev->constraints->max_uV)
-			ret = 0;
-	}
-
-	return ret;
+	return _regulator_list_voltage(regulator, selector, 1);
 }
 EXPORT_SYMBOL_GPL(regulator_list_voltage);
 
-- 
2.6.1

[PATCH 2/8] regulator: core: create unlocked version of regulator_set_voltage

[Sascha Hauer]

The unlocked version will be needed when we start propagating voltage
changes to the supply regulators.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/regulator/core.c | 57 +++++++++++++++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index bab426d..b814451 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2725,33 +2725,14 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	return ret;
 }
 
-/**
- * regulator_set_voltage - set regulator output voltage
- * @regulator: regulator source
- * @min_uV: Minimum required voltage in uV
- * @max_uV: Maximum acceptable voltage in uV
- *
- * Sets a voltage regulator to the desired output voltage. This can be set
- * during any regulator state. IOW, regulator can be disabled or enabled.
- *
- * If the regulator is enabled then the voltage will change to the new value
- * immediately otherwise if the regulator is disabled the regulator will
- * output at the new voltage when enabled.
- *
- * NOTE: If the regulator is shared between several devices then the lowest
- * request voltage that meets the system constraints will be used.
- * Regulator system constraints must be set for this regulator before
- * calling this function otherwise this call will fail.
- */
-int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
+static int regulator_set_voltage_unlocked(struct regulator *regulator,
+					  int min_uV, int max_uV)
 {
 	struct regulator_dev *rdev = regulator->rdev;
 	int ret = 0;
 	int old_min_uV, old_max_uV;
 	int current_uV;
 
-	mutex_lock(&rdev->mutex);
-
 	/* If we're setting the same range as last time the change
 	 * should be a noop (some cpufreq implementations use the same
 	 * voltage for multiple frequencies, for example).
@@ -2799,12 +2780,42 @@ int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
 		goto out2;
 
 out:
-	mutex_unlock(&rdev->mutex);
 	return ret;
 out2:
 	regulator->min_uV = old_min_uV;
 	regulator->max_uV = old_max_uV;
-	mutex_unlock(&rdev->mutex);
+
+	return ret;
+}
+
+/**
+ * regulator_set_voltage - set regulator output voltage
+ * @regulator: regulator source
+ * @min_uV: Minimum required voltage in uV
+ * @max_uV: Maximum acceptable voltage in uV
+ *
+ * Sets a voltage regulator to the desired output voltage. This can be set
+ * during any regulator state. IOW, regulator can be disabled or enabled.
+ *
+ * If the regulator is enabled then the voltage will change to the new value
+ * immediately otherwise if the regulator is disabled the regulator will
+ * output at the new voltage when enabled.
+ *
+ * NOTE: If the regulator is shared between several devices then the lowest
+ * request voltage that meets the system constraints will be used.
+ * Regulator system constraints must be set for this regulator before
+ * calling this function otherwise this call will fail.
+ */
+int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
+{
+	int ret = 0;
+
+	mutex_lock(&regulator->rdev->mutex);
+
+	ret = regulator_set_voltage_unlocked(regulator, min_uV, max_uV);
+
+	mutex_unlock(&regulator->rdev->mutex);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regulator_set_voltage);
-- 
2.6.1

[PATCH 3/8] regulator: introduce min_dropout_uv

[Sascha Hauer]

Linear voltage Regulators need a input voltage that is higher than the
output voltage. Allow to specify a minimum dropout voltage which will
be used later to find the best input voltage for regulators.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/linux/regulator/driver.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 4593222..a3815bd 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -245,6 +245,7 @@ enum regulator_type {
  * @linear_min_sel: Minimal selector for starting linear mapping
  * @fixed_uV: Fixed voltage of rails.
  * @ramp_delay: Time to settle down after voltage change (unit: uV/us)
+ * @min_dropout_uv: The minimum dropout voltage this regulator can handle
  * @linear_ranges: A constant table of possible voltage ranges.
  * @n_linear_ranges: Number of entries in the @linear_ranges table.
  * @volt_table: Voltage mapping table (if table based mapping)
@@ -292,6 +293,7 @@ struct regulator_desc {
 	unsigned int linear_min_sel;
 	int fixed_uV;
 	unsigned int ramp_delay;
+	int min_dropout_uv;
 
 	const struct regulator_linear_range *linear_ranges;
 	int n_linear_ranges;
-- 
2.6.1

[PATCH 4/8] regulator: core: introduce function to lock regulators and its supplies

[Sascha Hauer]

Each regulator_dev is locked with its own mutex. This is fine as long
as only one regulator_dev is locked, but makes lockdep unhappy when we
have to walk up the supply chain like it can happen in
regulator_get_voltage:

regulator_get_voltage ->
 mutex_lock(&regulator->rdev->mutex) ->
_regulator_get_voltage(regulator->rdev) ->
regulator_get_voltage(rdev->supply) ->
mutex_lock(&regulator->rdev->mutex);

This causes lockdep to issue a possible deadlock warning.

There are at least two ways to work around this:

- We can always lock the whole supply chain using the functions
  introduced with this patch.
- We could store the current voltage in struct regulator_rdev so
  that we do not have to walk up the supply chain for the
  _regulator_get_voltage case.

Anyway, regulator_lock_supply/regulator_unlock_supply will be needed
once we allow regulator_set_voltage to optimize the supply voltages.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/regulator/core.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b814451..bd66097 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -132,6 +132,45 @@ static bool have_full_constraints(void)
 }
 
 /**
+ * regulator_lock_supply - lock a regulator and its supplies
+ * @rdev:         regulator source
+ */
+static void regulator_lock_supply(struct regulator_dev *rdev)
+{
+	struct regulator *supply;
+	int i = 0;
+
+	while (1) {
+		mutex_lock_nested(&rdev->mutex, i++);
+		supply = rdev->supply;
+
+		if (!rdev->supply)
+			return;
+
+		rdev = supply->rdev;
+	}
+}
+
+/**
+ * regulator_unlock_supply - unlock a regulator and its supplies
+ * @rdev:         regulator source
+ */
+static void regulator_unlock_supply(struct regulator_dev *rdev)
+{
+	struct regulator *supply;
+
+	while (1) {
+		mutex_unlock(&rdev->mutex);
+		supply = rdev->supply;
+
+		if (!rdev->supply)
+			return;
+
+		rdev = supply->rdev;
+	}
+}
+
+/**
  * of_get_regulator - get a regulator device node based on supply name
  * @dev: Device pointer for the consumer (of regulator) device
  * @supply: regulator supply name
-- 
2.6.1

[PATCH 5/8] regulator: introduce regulator_get_voltage_floor

[Sascha Hauer]

Currently when we call set_voltage on a regulator we do not know
with which voltage we will end up with. Once we start with propagating
voltage changes to the supplies we have to know that before we start
doing anything. This patch introduces regulator_get_voltage_floor which
calculates the minimum voltage a regulator supports which is higher
than a given minimum voltage.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/regulator/core.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index bd66097..6623538 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2764,6 +2764,45 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	return ret;
 }
 
+/*
+ * Return the minimum voltage supported by a regulator that is higher or equal
+ * to a given voltage.
+ */
+static int regulator_get_voltage_floor(struct regulator *regulator, int min_uV)
+{
+	struct regulator_dev *rdev = regulator->rdev;
+	int num_voltages;
+	int best = INT_MAX;
+	int max_uV = INT_MAX;
+	int i, now, ret;
+
+	/* constraints check */
+	ret = regulator_check_voltage(rdev, &min_uV, &max_uV);
+	if (ret < 0)
+		return ret;
+
+	ret = regulator_check_consumers(rdev, &min_uV, &max_uV);
+	if (ret < 0)
+		return ret;
+
+	num_voltages = regulator_count_voltages(regulator);
+	if (num_voltages < 0)
+		return num_voltages;
+
+	for (i = 0; i < num_voltages; i++) {
+		now = _regulator_list_voltage(regulator, i, 0);
+		if (now < 0)
+			continue;
+		if (now < best && now >= min_uV)
+			best = now;
+	}
+
+	if (best > max_uV)
+		return -EINVAL;
+
+	return best;
+}
+
 static int regulator_set_voltage_unlocked(struct regulator *regulator,
 					  int min_uV, int max_uV)
 {
-- 
2.6.1

[PATCH 6/8] regulator: core: Propagate voltage changes to supply regulators

[Sascha Hauer]

Until now changing the voltage of a regulator only ever effected the
regulator itself, but never its supplies. It's a common pattern though
to put LDO regulators behind switching regulators. The switching
regulators efficiently drop the input voltage but have a high ripple on
their output. The output is then cleaned up by the LDOs. For higher
energy efficiency the voltage drop at the LDOs should be minimized. For
this scenario we need to propagate the voltage change to the supply
regulators. Another scenario where voltage propagation is desired is
a regulator which only consists of a switch and thus cannot regulate
voltages itself. In this case we can pass setting voltages to the
supply.

This patch adds support for voltage propagation. We do voltage
propagation when the current regulator has a minimum dropout voltage
specified or if the current regulator lacks a get_voltage operation
(indicating it's a switch and not a regulator).

Changing the supply voltage must be done carefully. When we are
increasing the current regulators output we must first increase the
supply voltage and then the regulator itself. When we are decreasing the
current regulators voltage we must decrease the supply voltage after
changing the current regulators voltage.

Calculating the optimum voltage for the supply regulator is a bit tricky
since the simple approach of just adding the desired minimum voltage and
the minimum dropout is not enough. It may happen that the current
regulator does not support the desired minimum voltage, but only a
higher one. This means we have to figure out the lowest voltage
supported by the regulator that is higher than the minimum desired
voltage. For this regulator_get_voltage_floor is used.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/regulator/core.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 6623538..a01f833 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2810,6 +2810,8 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 	int ret = 0;
 	int old_min_uV, old_max_uV;
 	int current_uV;
+	int best_supply_uV = 0;
+	int supply_change_uV = 0;
 
 	/* If we're setting the same range as last time the change
 	 * should be a noop (some cpufreq implementations use the same
@@ -2853,10 +2855,51 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 	if (ret < 0)
 		goto out2;
 
+	if (rdev->supply && (rdev->desc->min_dropout_uv ||
+				!rdev->desc->ops->get_voltage)) {
+		int current_supply_uV;
+
+		best_supply_uV = regulator_get_voltage_floor(regulator, min_uV);
+		if (best_supply_uV < 0) {
+			ret = best_supply_uV;
+			goto out2;
+		}
+
+		best_supply_uV += rdev->desc->min_dropout_uv;
+
+		current_supply_uV = _regulator_get_voltage(rdev->supply->rdev);
+		if (current_supply_uV < 0) {
+			ret = current_supply_uV;
+			goto out2;
+		}
+
+		supply_change_uV = best_supply_uV - current_supply_uV;
+	}
+
+	if (supply_change_uV > 0) {
+		ret = regulator_set_voltage_unlocked(rdev->supply,
+				best_supply_uV, INT_MAX);
+		if (ret) {
+			dev_err(&rdev->dev, "Failed to increase supply voltage: %d\n",
+					ret);
+			goto out2;
+		}
+	}
+
 	ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
 	if (ret < 0)
 		goto out2;
 
+	if (supply_change_uV < 0) {
+		ret = regulator_set_voltage_unlocked(rdev->supply,
+				best_supply_uV, INT_MAX);
+		if (ret)
+			dev_warn(&rdev->dev, "Failed to decrease supply voltage: %d\n",
+					ret);
+		/* No need to fail here */
+		ret = 0;
+	}
+
 out:
 	return ret;
 out2:
@@ -2888,11 +2931,11 @@ int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
 {
 	int ret = 0;
 
-	mutex_lock(&regulator->rdev->mutex);
+	regulator_lock_supply(regulator->rdev);
 
 	ret = regulator_set_voltage_unlocked(regulator, min_uV, max_uV);
 
-	mutex_unlock(&regulator->rdev->mutex);
+	regulator_unlock_supply(regulator->rdev);
 
 	return ret;
 }
-- 
2.6.1

[PATCH 7/8] regulator: i.MX anatop: Allow supply regulator

[Sascha Hauer]

The anatop regulators are SoC internal LDO regulators usually supplied
by an external PMIC. This patch adds support for specifying the supply
from the device tree using the vin-supply property.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 Documentation/devicetree/bindings/regulator/anatop-regulator.txt | 1 +
 drivers/regulator/anatop-regulator.c                             | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/anatop-regulator.txt b/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
index 758eae2..37c4ea0 100644
--- a/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
@@ -13,6 +13,7 @@ Optional properties:
 - anatop-delay-reg-offset: Anatop MFD step time register offset
 - anatop-delay-bit-shift: Bit shift for the step time register
 - anatop-delay-bit-width: Number of bits used in the step time register
+- vin-supply: The supply for this regulator
 
 Any property defined as part of the core regulator
 binding, defined in regulator.txt, can also be used.
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 52ea605..ba78870 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -30,6 +30,7 @@
 #include <linux/regmap.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/of_regulator.h>
+#include <linux/regulator/machine.h>
 
 #define LDO_RAMP_UP_UNIT_IN_CYCLES      64 /* 64 cycles per step */
 #define LDO_RAMP_UP_FREQ_IN_MHZ         24 /* cycle based on 24M OSC */
@@ -199,6 +200,7 @@ static int anatop_regulator_probe(struct platform_device *pdev)
 	rdesc->owner = THIS_MODULE;
 
 	initdata = of_get_regulator_init_data(dev, np, rdesc);
+	initdata->supply_regulator = "vin";
 	sreg->initdata = initdata;
 
 	anatop_np = of_get_parent(np);
@@ -262,6 +264,7 @@ static int anatop_regulator_probe(struct platform_device *pdev)
 	rdesc->vsel_reg = sreg->control_reg;
 	rdesc->vsel_mask = ((1 << sreg->vol_bit_width) - 1) <<
 			   sreg->vol_bit_shift;
+	rdesc->min_dropout_uv = 125000;
 
 	config.dev = &pdev->dev;
 	config.init_data = initdata;
-- 
2.6.1

[PATCH 8/8] ARM: i.MX6 Phytec PFLA02: Add supplies for the SoC internal regulators

[Sascha Hauer]

The SoC internal regulators for the CPU and the SoC come from the
DA9063 vdd_core and vdd_soc. Add this relationship to the device tree
so that the voltage drop on the SoC internal LDO regulators can be
minimized.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Acked-by: Mark Brown <broonie@kernel.org>
---
 arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi
index 9e6ecd9..f19c680 100644
--- a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi
@@ -379,6 +379,18 @@
 	status = "disabled";
 };
 
+&reg_arm {
+	vin-supply = <&vddcore_reg>;
+};
+
+&reg_pu {
+	vin-supply = <&vddsoc_reg>;
+};
+
+&reg_soc {
+	vin-supply = <&vddsoc_reg>;
+};
+
 &uart3 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart3>;
-- 
2.6.1

Re: [PATCH 1/8] regulator: core: create unlocked version of regulator_list_voltage

[Mark Brown]

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

On Tue, Oct 13, 2015 at 12:45:24PM +0200, Sascha Hauer wrote:
> The unlocked version will be needed when we start propagating voltage
> changes to the supply regulators.

Please don't resend already applied patches, if there are any changes
that need making send incremental changes on top of what's already
applied.

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

Re: [PATCH 3/8] regulator: introduce min_dropout_uv

[Mark Brown]

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

On Tue, Oct 13, 2015 at 12:45:26PM +0200, Sascha Hauer wrote:

>   * @fixed_uV: Fixed voltage of rails.
>   * @ramp_delay: Time to settle down after voltage change (unit: uV/us)
> + * @min_dropout_uv: The minimum dropout voltage this regulator can handle

This should be uV - we've always done that since it looks so wrong
otherwise.  I'll correct this as a followup.

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

Applied "regulator: introduce min_dropout_uv" to the regulator tree

[Mark Brown]

The patch

   regulator: introduce min_dropout_uv

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From cfef37071f2bd32162f3b988742caab87192389c Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Tue, 13 Oct 2015 12:45:26 +0200
Subject: [PATCH] regulator: introduce min_dropout_uv

Linear voltage Regulators need a input voltage that is higher than the
output voltage. Allow to specify a minimum dropout voltage which will
be used later to find the best input voltage for regulators.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 include/linux/regulator/driver.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 45932228cbf5..a3815bde342b 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -245,6 +245,7 @@ enum regulator_type {
  * @linear_min_sel: Minimal selector for starting linear mapping
  * @fixed_uV: Fixed voltage of rails.
  * @ramp_delay: Time to settle down after voltage change (unit: uV/us)
+ * @min_dropout_uv: The minimum dropout voltage this regulator can handle
  * @linear_ranges: A constant table of possible voltage ranges.
  * @n_linear_ranges: Number of entries in the @linear_ranges table.
  * @volt_table: Voltage mapping table (if table based mapping)
@@ -292,6 +293,7 @@ struct regulator_desc {
 	unsigned int linear_min_sel;
 	int fixed_uV;
 	unsigned int ramp_delay;
+	int min_dropout_uv;
 
 	const struct regulator_linear_range *linear_ranges;
 	int n_linear_ranges;
-- 
2.6.1

Re: [PATCH 5/8] regulator: introduce regulator_get_voltage_floor

[Mark Brown]

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

On Tue, Oct 13, 2015 at 12:45:28PM +0200, Sascha Hauer wrote:

> +	num_voltages = regulator_count_voltages(regulator);
> +	if (num_voltages < 0)
> +		return num_voltages;

> +	for (i = 0; i < num_voltages; i++) {
> +		now = _regulator_list_voltage(regulator, i, 0);
> +		if (now < 0)
> +			continue;
> +		if (now < best && now >= min_uV)
> +			best = now;
> +	}

Why is this not a factoring out of existing code (indeed it is itself a
reimplementation of regulator_map_voltage_iterate())?  This will also be
a substantial performance loss in cases where we have a known mapping
function - we should use a map_voltage() operation if one exists like we
do in _do_set_voltage().  That has logic to handle missing mapping
functions as a transition measure, now I look at it we should probably
remove that code and just require that the mapping function is set if
appropriate.

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

Re: [PATCH 6/8] regulator: core: Propagate voltage changes to supply regulators

[Mark Brown]

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

On Tue, Oct 13, 2015 at 12:45:29PM +0200, Sascha Hauer wrote:

> +		best_supply_uV = regulator_get_voltage_floor(regulator, min_uV);
> +		if (best_supply_uV < 0) {
> +			ret = best_supply_uV;
> +			goto out2;
> +		}

Now I look at the user here this is just the map voltage operation.  We
could even refactor...

>  	ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
>  	if (ret < 0)
>  		goto out2;

...do_set_voltage() so that we only do the mapping once, though that
gets tricky as we still support devices that don't have mapping
configured :/ .

Otherwise this looks good.

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

Re: Applied "regulator: introduce min_dropout_uv" to the regulator tree

[Mark Brown]

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

On Fri, Oct 16, 2015 at 05:41:17PM +0100, Mark Brown wrote:
> The patch
> 
>    regulator: introduce min_dropout_uv

Actually since I'm punting on the change to use this I'll just amend the
commit rather than fix incrementally.

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

Applied "regulator: core: create unlocked version of regulator_set_voltage" to the regulator tree

[Mark Brown]

The patch

   regulator: core: create unlocked version of regulator_set_voltage

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From a9f226bcd9bb1941e581806e83d2c03d4043c367 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Tue, 13 Oct 2015 12:45:25 +0200
Subject: [PATCH] regulator: core: create unlocked version of
 regulator_set_voltage

The unlocked version will be needed when we start propagating voltage
changes to the supply regulators.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 57 +++++++++++++++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f28c133693ad..5d161e11d5e6 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2761,33 +2761,14 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	return ret;
 }
 
-/**
- * regulator_set_voltage - set regulator output voltage
- * @regulator: regulator source
- * @min_uV: Minimum required voltage in uV
- * @max_uV: Maximum acceptable voltage in uV
- *
- * Sets a voltage regulator to the desired output voltage. This can be set
- * during any regulator state. IOW, regulator can be disabled or enabled.
- *
- * If the regulator is enabled then the voltage will change to the new value
- * immediately otherwise if the regulator is disabled the regulator will
- * output at the new voltage when enabled.
- *
- * NOTE: If the regulator is shared between several devices then the lowest
- * request voltage that meets the system constraints will be used.
- * Regulator system constraints must be set for this regulator before
- * calling this function otherwise this call will fail.
- */
-int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
+static int regulator_set_voltage_unlocked(struct regulator *regulator,
+					  int min_uV, int max_uV)
 {
 	struct regulator_dev *rdev = regulator->rdev;
 	int ret = 0;
 	int old_min_uV, old_max_uV;
 	int current_uV;
 
-	mutex_lock(&rdev->mutex);
-
 	/* If we're setting the same range as last time the change
 	 * should be a noop (some cpufreq implementations use the same
 	 * voltage for multiple frequencies, for example).
@@ -2835,12 +2816,42 @@ int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
 		goto out2;
 
 out:
-	mutex_unlock(&rdev->mutex);
 	return ret;
 out2:
 	regulator->min_uV = old_min_uV;
 	regulator->max_uV = old_max_uV;
-	mutex_unlock(&rdev->mutex);
+
+	return ret;
+}
+
+/**
+ * regulator_set_voltage - set regulator output voltage
+ * @regulator: regulator source
+ * @min_uV: Minimum required voltage in uV
+ * @max_uV: Maximum acceptable voltage in uV
+ *
+ * Sets a voltage regulator to the desired output voltage. This can be set
+ * during any regulator state. IOW, regulator can be disabled or enabled.
+ *
+ * If the regulator is enabled then the voltage will change to the new value
+ * immediately otherwise if the regulator is disabled the regulator will
+ * output at the new voltage when enabled.
+ *
+ * NOTE: If the regulator is shared between several devices then the lowest
+ * request voltage that meets the system constraints will be used.
+ * Regulator system constraints must be set for this regulator before
+ * calling this function otherwise this call will fail.
+ */
+int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
+{
+	int ret = 0;
+
+	mutex_lock(&regulator->rdev->mutex);
+
+	ret = regulator_set_voltage_unlocked(regulator, min_uV, max_uV);
+
+	mutex_unlock(&regulator->rdev->mutex);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regulator_set_voltage);
-- 
2.6.1

Re: [PATCH 5/8] regulator: introduce regulator_get_voltage_floor

[Sascha Hauer]

On Fri, Oct 16, 2015 at 05:50:23PM +0100, Mark Brown wrote:
> On Tue, Oct 13, 2015 at 12:45:28PM +0200, Sascha Hauer wrote:
> 
> > +	num_voltages = regulator_count_voltages(regulator);
> > +	if (num_voltages < 0)
> > +		return num_voltages;
> 
> > +	for (i = 0; i < num_voltages; i++) {
> > +		now = _regulator_list_voltage(regulator, i, 0);
> > +		if (now < 0)
> > +			continue;
> > +		if (now < best && now >= min_uV)
> > +			best = now;
> > +	}
> 
> Why is this not a factoring out of existing code (indeed it is itself a
> reimplementation of regulator_map_voltage_iterate())?  This will also be
> a substantial performance loss in cases where we have a known mapping
> function - we should use a map_voltage() operation if one exists like we
> do in _do_set_voltage().  That has logic to handle missing mapping
> functions as a transition measure, now I look at it we should probably
> remove that code and just require that the mapping function is set if
> appropriate.

I didn't realize the map_voltage functionality is exactly what I want to
have. Turns out that I can factor out a regulator_map_voltage() function
instead of creating a regulator_get_voltage_floor() function. Updated
series follows shortly.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |