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

Groups > linux.kernel > #1245576 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] regulator: Propagate voltage changes to supply regulators Sascha Hauer <s.hauer@pengutronix.de> - 2015-10-13 12:50 +0200
    [PATCH 4/8] regulator: core: introduce function to lock regulators and its supplies Sascha Hauer <s.hauer@pengutronix.de> - 2015-10-13 12:50 +0200
    [PATCH 3/8] regulator: introduce min_dropout_uv Sascha Hauer <s.hauer@pengutronix.de> - 2015-10-13 12:50 +0200
      Re: [PATCH 3/8] regulator: introduce min_dropout_uv Mark Brown <broonie@kernel.org> - 2015-10-16 18:20 +0200
      Applied "regulator: introduce min_dropout_uv" to the regulator tree Mark Brown <broonie@kernel.org> - 2015-10-16 18:50 +0200
        Re: Applied "regulator: introduce min_dropout_uv" to the regulator  tree Mark Brown <broonie@kernel.org> - 2015-10-16 19:00 +0200
    [PATCH 6/8] regulator: core: Propagate voltage changes to supply regulators Sascha Hauer <s.hauer@pengutronix.de> - 2015-10-13 12:50 +0200
      Re: [PATCH 6/8] regulator: core: Propagate voltage changes to supply  regulators Mark Brown <broonie@kernel.org> - 2015-10-16 19:00 +0200

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1245578 — [PATCH 3/8] 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>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1248955 — Re: [PATCH 3/8] regulator: introduce min_dropout_uv

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

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.

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

#1248975 — Applied "regulator: introduce min_dropout_uv" to the regulator tree

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1248983 — Re: Applied "regulator: introduce min_dropout_uv" to the regulator tree

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

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.

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

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

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

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

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web