[PATCH v5 0/5] Add coupled regulators mechanism

[PATCH v5 0/5] Add coupled regulators mechanism

[Maciej Purski]

Hi all,

this patchset adds a new mechanism to the framework - regulators' coupling.

On Odroid XU3/4 and other Exynos5422 based boards there is a case, that
different devices on the board are supplied by different regulators
with non-fixed voltages. If one of these devices temporarily requires 
higher voltage, there might occur a situation that the spread between 
devices' voltages is so high, that there is a risk of changing
'high' and 'low' states on the interconnection between devices powered
by those regulators. 

Algorithmicaly the problem was solved by: 
Inderpal Singh <inderpal.s@samsung.com>
Doug Anderson <dianders@chromium.org>

The discussion on that subject can be found here:
https://lkml.org/lkml/2014/4/29/28

Therefore this patchset is an attempt to apply the idea to regulators core
as concluded in the discussion by Mark Brown and Doug Anderson.

This feature is required to enable support for generic CPUfreq 
and devfreq drivers for the mentioned boards. 

Note on the locking model:
When balancing voltage of a group of coupled regulators, we lock all
of them for the whole operation. When voltage of an individual regulator
is about to change, its suppliers are additionally locked.

The current assumption is that an uncoupled regulator is a special case
of a coupled one, so they should share a common voltage setting path.


Best regards,

	Maciej Purski

---
Changes in v5:
- rebase against current Mark Brown's regulators next

Changes in v4:
- make paths for coupled and uncoupled regulators common
- coupling descriptors are now always present in regulator_dev
- fail to probe if data inconsistency is detected
- retry to resolve coupling regultors in late init call
- rebase on top of linux-next 20180119
- fix commit messages
- split patches to make the patchset easier to review

Changes in v3:
- move dts parsing code to of_regulator.c, in order to the so,
  add a new commit in which of_regulator_find_by_node() is moved
  to of_regulator.c as well
- improve error messages
- move max_spread variable to constraints
- perform resolving of coupled regulators under a list mutex
- remove useless locking functions
- some minor refactorization
- improve commit messages

Changes in RFC v2:
- allow coupling n regulators (in fact up to constant value, now
  set to 10)
- change algorithm to be more readable
- introduce better locking
- add more comments
- split first patch into two
- update commit messages
- change sequence of the patches

Maciej Purski (5):
  regulator: bindings: Add properties for coupled regulators
  regulator: core: Parse coupled regulators properties
  regulator: core: Resolve coupled regulators
  regulator: core: Add voltage balancing mechanism
  regulator: core: Change voltage setting path

 .../devicetree/bindings/regulator/regulator.txt    |   5 +
 drivers/regulator/core.c                           | 381 ++++++++++++++++++++-
 drivers/regulator/internal.h                       |   6 +
 drivers/regulator/of_regulator.c                   | 149 ++++++++
 include/linux/regulator/driver.h                   |  18 +
 include/linux/regulator/machine.h                  |   4 +
 6 files changed, 545 insertions(+), 18 deletions(-)

-- 
2.7.4

[PATCH v5 1/5] regulator: bindings: Add properties for coupled regulators

[Maciej Purski]

Some regulators require keeping their voltage spread below defined
max_spread.

Add properties to provide information on regulators' coupling.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 Documentation/devicetree/bindings/regulator/regulator.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index 2babe15b..62510d6 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -68,6 +68,11 @@ Optional properties:
 	0: Disable active discharge.
 	1: Enable active discharge.
 	Absence of this property will leave configuration to default.
+- regulator-coupled-with: Regulators with which the regulator
+  is coupled. The linkage is 2-way - all coupled regulators should be linked
+  with each other.
+- regulator-coupled-max-spread: Max spread between voltages of coupled regulators
+  in microvolts.
 
 Deprecated properties:
 - regulator-compatible: If a regulator chip contains multiple
-- 
2.7.4

[PATCH v5 2/5] regulator: core: Parse coupled regulators properties

[Maciej Purski]

On Odroid XU3/4 and other Exynos5422 based boards there is a case, that
different devices on the board are supplied by different regulators
with non-fixed voltages. If one of these devices temporarily requires
higher voltage, there might occur a situation that the spread between
devices' voltages is so high, that there is a risk of changing
'high' and 'low' states on the interconnection between devices powered
by those regulators.

Add new structure "coupling_desc" to regulator_dev, which contains
pointers to all coupled regulators including the owner of the structure,
number of coupled regulators and counter of currently resolved
regulators.

Add of_functions to parse all data needed in regulator coupling.
Provide method to check DTS data consistency. Check if each coupled
regulator's max_spread is equal and if their lists of regulators match.

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/regulator/internal.h      |   6 ++
 drivers/regulator/of_regulator.c  | 149 ++++++++++++++++++++++++++++++++++++++
 include/linux/regulator/driver.h  |  18 +++++
 include/linux/regulator/machine.h |   4 +
 4 files changed, 177 insertions(+)

diff --git a/drivers/regulator/internal.h b/drivers/regulator/internal.h
index abfd56e..f253a47 100644
--- a/drivers/regulator/internal.h
+++ b/drivers/regulator/internal.h
@@ -84,4 +84,10 @@ enum regulator_get_type {
 struct regulator *_regulator_get(struct device *dev, const char *id,
 				 enum regulator_get_type get_type);
 
+struct regulator_dev *of_parse_coupled_regulator(struct regulator_dev *rdev,
+						 int index);
+
+int of_get_n_coupled(struct regulator_dev *rdev);
+
+bool of_check_coupling_data(struct regulator_dev *rdev);
 #endif
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index f47264f..cf3210c 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -138,6 +138,10 @@ static void of_get_regulation_constraints(struct device_node *np,
 	if (!of_property_read_u32(np, "regulator-system-load", &pval))
 		constraints->system_load = pval;
 
+	if (!of_property_read_u32(np, "regulator-coupled-max-spread",
+				  &pval))
+		constraints->max_spread = pval;
+
 	constraints->over_current_protection = of_property_read_bool(np,
 					"regulator-over-current-protection");
 
@@ -407,3 +411,148 @@ struct regulator_dev *of_find_regulator_by_node(struct device_node *np)
 
 	return dev ? dev_to_rdev(dev) : NULL;
 }
+
+/*
+ * Returns number of regulators coupled with rdev.
+ */
+int of_get_n_coupled(struct regulator_dev *rdev)
+{
+	struct device_node *node = rdev->dev.of_node;
+	int n_phandles;
+
+	n_phandles = of_count_phandle_with_args(node,
+						"regulator-coupled-with", 0);
+
+	return (n_phandles > 0) ? n_phandles : 0;
+}
+
+/* Looks for "to_find" device_node in src's "regulator-coupled-with" property */
+static bool of_coupling_find_node(struct device_node *src,
+				  struct device_node *to_find)
+{
+	int n_phandles, i;
+	bool found = false;
+
+	n_phandles = of_count_phandle_with_args(src,
+						"regulator-coupled-with", 0);
+
+	for (i = 0; i < n_phandles; i++) {
+		struct device_node *tmp = of_parse_phandle(src,
+					   "regulator-coupled-with", i);
+
+		if (!tmp)
+			break;
+
+		/* found */
+		if (tmp == to_find)
+			found = true;
+
+		of_node_put(tmp);
+
+		if (found)
+			break;
+	}
+
+	return found;
+}
+
+/**
+ * of_check_coupling_data - Parse rdev's coupling properties and check data
+ *			    consistency
+ * @rdev - pointer to regulator_dev whose data is checked
+ *
+ * Function checks if all the following conditions are met:
+ * - rdev's max_spread is greater than 0
+ * - all coupled regulators have the same max_spread
+ * - all coupled regulators have the same number of regulator_dev phandles
+ * - all regulators are linked to each other
+ *
+ * Returns true if all conditions are met.
+ */
+bool of_check_coupling_data(struct regulator_dev *rdev)
+{
+	int max_spread = rdev->constraints->max_spread;
+	struct device_node *node = rdev->dev.of_node;
+	int n_phandles = of_get_n_coupled(rdev);
+	struct device_node *c_node;
+	int i;
+	bool ret = true;
+
+	if (max_spread <= 0) {
+		dev_err(&rdev->dev, "max_spread value invalid\n");
+		return false;
+	}
+
+	/* iterate over rdev's phandles */
+	for (i = 0; i < n_phandles; i++) {
+		int c_max_spread, c_n_phandles;
+
+		c_node = of_parse_phandle(node,
+					  "regulator-coupled-with", i);
+
+		if (!c_node)
+			ret = false;
+
+		c_n_phandles = of_count_phandle_with_args(c_node,
+							  "regulator-coupled-with",
+							  0);
+
+		if (c_n_phandles != n_phandles) {
+			dev_err(&rdev->dev, "number of couped reg phandles mismatch\n");
+			ret = false;
+			goto clean;
+		}
+
+		if (of_property_read_u32(c_node, "regulator-coupled-max-spread",
+					 &c_max_spread)) {
+			ret = false;
+			goto clean;
+		}
+
+		if (c_max_spread != max_spread) {
+			dev_err(&rdev->dev,
+				"coupled regulators max_spread mismatch\n");
+			ret = false;
+			goto clean;
+		}
+
+		if (!of_coupling_find_node(c_node, node)) {
+			dev_err(&rdev->dev, "missing 2-way linking for coupled regulators\n");
+			ret = false;
+		}
+
+clean:
+		of_node_put(c_node);
+		if (!ret)
+			break;
+	}
+
+	return ret;
+}
+
+/**
+ * of_parse_coupled regulator - Get regulator_dev pointer from rdev's property
+ * @rdev: Pointer to regulator_dev, whose DTS is used as a source to parse
+ *	  "regulator-coupled-with" property
+ * @index: Index in phandles array
+ *
+ * Returns the regulator_dev pointer parsed from DTS. If it has not been yet
+ * registered, returns NULL
+ */
+struct regulator_dev *of_parse_coupled_regulator(struct regulator_dev *rdev,
+						 int index)
+{
+	struct device_node *node = rdev->dev.of_node;
+	struct device_node *c_node;
+	struct regulator_dev *c_rdev;
+
+	c_node = of_parse_phandle(node, "regulator-coupled-with", index);
+	if (!c_node)
+		return NULL;
+
+	c_rdev = of_find_regulator_by_node(c_node);
+
+	of_node_put(c_node);
+
+	return c_rdev;
+}
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 4fc96cb..e0f5d4c 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -15,6 +15,8 @@
 #ifndef __LINUX_REGULATOR_DRIVER_H_
 #define __LINUX_REGULATOR_DRIVER_H_
 
+#define MAX_COUPLED		10
+
 #include <linux/device.h>
 #include <linux/notifier.h>
 #include <linux/regulator/consumer.h>
@@ -407,6 +409,20 @@ struct regulator_config {
 };
 
 /*
+ * struct coupling_desc
+ *
+ * Describes coupling of regulators. Each regulator should have
+ * at least a pointer to itself in coupled_rdevs array.
+ * When a new coupled regulator is resolved, n_resolved is
+ * incremented.
+ */
+struct coupling_desc {
+	struct regulator_dev *coupled_rdevs[MAX_COUPLED];
+	int n_resolved;
+	int n_coupled;
+};
+
+/*
  * struct regulator_dev
  *
  * Voltage / Current regulator class device. One for each
@@ -429,6 +445,8 @@ struct regulator_dev {
 	/* lists we own */
 	struct list_head consumer_list; /* consumers we supply */
 
+	struct coupling_desc coupling_desc;
+
 	struct blocking_notifier_head notifier;
 	struct mutex mutex; /* consumer lock */
 	struct module *owner;
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index 93a0489..3468703 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -103,6 +103,7 @@ struct regulator_state {
  * @ilim_uA: Maximum input current.
  * @system_load: Load that isn't captured by any consumer requests.
  *
+ * @max_spread: Max possible spread between coupled regulators
  * @valid_modes_mask: Mask of modes which may be configured by consumers.
  * @valid_ops_mask: Operations which may be performed by consumers.
  *
@@ -154,6 +155,9 @@ struct regulation_constraints {
 
 	int system_load;
 
+	/* used for coupled regulators */
+	int max_spread;
+
 	/* valid regulator operating modes for this machine */
 	unsigned int valid_modes_mask;
 
-- 
2.7.4

[PATCH v5 3/5] regulator: core: Resolve coupled regulators

[Maciej Purski]

On Odroid XU3/4 and other Exynos5422 based boards there is a case, that
different devices on the board are supplied by different regulators
with non-fixed voltages. If one of these devices temporarily requires
higher voltage, there might occur a situation that the spread between
two devices' voltages is so high, that there is a risk of changing
'high' and 'low' states on the interconnection between devices powered
by those regulators.

Fill coupling descriptor with data obtained from DTS using previously
defined of_functions. Fail to register a regulator, if some data
inconsistency occurs. If some coupled regulators are not yet registered,
don't fail to register, but try to resolve them in late init call.

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/regulator/core.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 5494189..f8adfe4 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4067,6 +4067,88 @@ static int regulator_register_resolve_supply(struct device *dev, void *data)
 	return 0;
 }
 
+static int regulator_fill_coupling_array(struct regulator_dev *rdev)
+{
+	struct coupling_desc *c_desc = &rdev->coupling_desc;
+	int n_coupled = c_desc->n_coupled;
+	struct regulator_dev *c_rdev;
+	int i;
+
+	for (i = 1; i < n_coupled; i++) {
+		/* already resolved */
+		if (c_desc->coupled_rdevs[i])
+			continue;
+
+		c_rdev = of_parse_coupled_regulator(rdev, i - 1);
+
+		if (c_rdev) {
+			c_desc->coupled_rdevs[i] = c_rdev;
+			c_desc->n_resolved++;
+		}
+	}
+
+	if (rdev->coupling_desc.n_resolved < n_coupled)
+		return -1;
+	else
+		return 0;
+}
+
+static int regulator_register_fill_coupling_array(struct device *dev,
+						  void *data)
+{
+	struct regulator_dev *rdev = dev_to_rdev(dev);
+
+	if (regulator_fill_coupling_array(rdev))
+		rdev_dbg(rdev, "unable to resolve coupling\n");
+
+	return 0;
+}
+
+static int regulator_resolve_coupling(struct regulator_dev *rdev)
+{
+	int n_phandles = of_get_n_coupled(rdev);
+
+	if (n_phandles + 1 > MAX_COUPLED) {
+		rdev_err(rdev, "too many regulators coupled\n");
+		return -EPERM;
+	}
+
+	/*
+	 * Every regulator should always have coupling descriptor filled with
+	 * at least pointer to itself.
+	 */
+	rdev->coupling_desc.coupled_rdevs[0] = rdev;
+	rdev->coupling_desc.n_coupled = n_phandles + 1;
+	rdev->coupling_desc.n_resolved++;
+
+	/* regulator isn't coupled */
+	if (n_phandles == 0)
+		return 0;
+
+	/* regulator, which can't change its voltage, can't be coupled */
+	if (!regulator_ops_is_valid(rdev, REGULATOR_CHANGE_VOLTAGE)) {
+		rdev_err(rdev, "voltage operation not allowed\n");
+		return -EPERM;
+	}
+
+	if (rdev->constraints->max_spread <= 0) {
+		rdev_err(rdev, "wrong max_spread value\n");
+		return -EPERM;
+	}
+
+	if (!of_check_coupling_data(rdev))
+		return -EPERM;
+
+	/*
+	 * After everything has been checked, try to fill rdevs array
+	 * with pointers to regulators parsed from device tree. If some
+	 * regulators are not registered yet, retry in late init call
+	 */
+	regulator_fill_coupling_array(rdev);
+
+	return 0;
+}
+
 /**
  * regulator_register - register regulator
  * @regulator_desc: regulator to register
@@ -4200,6 +4282,13 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	if (ret < 0)
 		goto wash;
 
+	mutex_lock(&regulator_list_mutex);
+	ret = regulator_resolve_coupling(rdev);
+	mutex_unlock(&regulator_list_mutex);
+
+	if (ret != 0)
+		goto wash;
+
 	/* add consumers devices */
 	if (init_data) {
 		mutex_lock(&regulator_list_mutex);
@@ -4693,6 +4782,9 @@ static int __init regulator_init_complete(void)
 	class_for_each_device(&regulator_class, NULL, NULL,
 			      regulator_late_cleanup);
 
+	class_for_each_device(&regulator_class, NULL, NULL,
+			      regulator_register_fill_coupling_array);
+
 	return 0;
 }
 late_initcall_sync(regulator_init_complete);
-- 
2.7.4

[PATCH v5 4/5] regulator: core: Add voltage balancing mechanism

[Maciej Purski]

On Odroid XU3/4 and other Exynos5422 based boards there is a case, that
different devices on the board are supplied by different regulators
with non-fixed voltages. If one of these devices temporarily requires
higher voltage, there might occur a situation that the spread between
two devices' voltages is so high, that there is a risk of changing
'high' and 'low' states on the interconnection between devices powered
by those regulators.

Introduce new function regulator_balance_voltage(), which
keeps max_spread constraint fulfilled between a group of coupled
regulators. It should be called if a regulator changes its
voltage or after disabling or enabling. Disabled regulators should
follow changes of the enabled ones, but their consumers' demands
shouldn't be taken into account while calculating voltage of other
coupled regulators.

Find voltages, which are closest to suiting all the consumers' demands,
while fulfilling max_spread constraint, keeping the following rules:
- if one regulator is about to rise its voltage, rise others
  voltages in order to keep the max_spread
- if a regulator, which has caused rising other regulators, is
  lowered, lower other regulators if possible
- if one regulator is about to lower its voltage, but it hasn't caused
  rising other regulators, don't change its voltage if it breaks the
  max_spread

Change regulators' voltages step by step, keeping max_spread constraint
fulfilled all the time. Function regulator_get_optimal_voltage()
should find the best possible change for the regulator, which doesn't
break max_spread constraint. In function regulator_balance_voltage()
optimize number of steps by finding highest voltage difference on
each iteration.

If a regulator, which is about to change its voltage, is not coupled,
method regulator_get_optimal_voltage() should simply return the lowest
voltage fulfilling consumers' demands.

Coupling should be checked only if the system is in PM_SUSPEND_ON state.

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/regulator/core.c | 192 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 192 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f8adfe4..b24a987 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -105,6 +105,8 @@ static int _notifier_call_chain(struct regulator_dev *rdev,
 				  unsigned long event, void *data);
 static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 				     int min_uV, int max_uV);
+static int regulator_balance_voltage(struct regulator_dev *rdev,
+				     suspend_state_t state);
 static struct regulator *create_regulator(struct regulator_dev *rdev,
 					  struct device *dev,
 					  const char *supply_name);
@@ -3040,6 +3042,196 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 	return ret;
 }
 
+static int regulator_get_optimal_voltage(struct regulator_dev *rdev)
+{
+	struct coupling_desc *c_desc = &rdev->coupling_desc;
+	struct regulator_dev **c_rdevs = c_desc->coupled_rdevs;
+	int max_spread = rdev->constraints->max_spread;
+	int n_coupled = c_desc->n_coupled;
+	int desired_min_uV, desired_max_uV, min_current_uV = INT_MAX;
+	int max_current_uV = 0, highest_min_uV = 0, target_uV, possible_uV;
+	int i, ret;
+
+	/* If consumers don't provide any demands, set voltage to min_uV */
+	desired_min_uV = rdev->constraints->min_uV;
+	desired_max_uV = rdev->constraints->max_uV;
+	ret = regulator_check_consumers(rdev,
+					&desired_min_uV,
+					&desired_max_uV, PM_SUSPEND_ON);
+	if (ret < 0)
+		goto out;
+
+	/*
+	 * If there are no coupled regulators, simply set the voltage demanded
+	 * by consumers.
+	 */
+	if (n_coupled == 1) {
+		ret = desired_min_uV;
+		goto out;
+	}
+
+	/* Find highest min desired voltage */
+	for (i = 0; i < n_coupled; i++) {
+		int tmp_min = 0;
+		int tmp_max = INT_MAX;
+
+		if (!_regulator_is_enabled(c_rdevs[i]))
+			continue;
+
+		ret = regulator_check_consumers(c_rdevs[i],
+						&tmp_min,
+						&tmp_max, PM_SUSPEND_ON);
+		if (ret < 0)
+			goto out;
+
+		if (tmp_min > highest_min_uV)
+			highest_min_uV = tmp_min;
+	}
+
+	/*
+	 * Let target_uV be equal to the desired one if possible.
+	 * If not, set it to minimum voltage, allowed by other coupled
+	 * regulators.
+	 */
+	target_uV = max(desired_min_uV,  highest_min_uV - max_spread);
+
+	/*
+	 * Find min and max voltages, which currently aren't
+	 * violating max_spread
+	 */
+	for (i = 0; i < n_coupled; i++) {
+		int tmp_act;
+
+		/*
+		 * Don't check the regulator, which is about
+		 * to change voltage
+		 */
+		if (c_rdevs[i] == rdev)
+			continue;
+		if (!_regulator_is_enabled(c_rdevs[i]))
+			continue;
+
+		tmp_act = _regulator_get_voltage(c_rdevs[i]);
+		if (tmp_act < 0) {
+			ret = tmp_act;
+			goto out;
+		}
+
+		if (tmp_act < min_current_uV)
+			min_current_uV = tmp_act;
+
+		if (tmp_act > max_current_uV)
+			max_current_uV = tmp_act;
+	}
+
+	/* There aren't any other regulators enabled */
+	if (max_current_uV == 0) {
+		possible_uV = target_uV;
+	} else {
+		/*
+		 * Correct target voltage, so as it currently isn't
+		 * violating max_spread
+		 */
+		possible_uV = max(target_uV, max_current_uV - max_spread);
+		possible_uV = min(possible_uV, min_current_uV + max_spread);
+	}
+
+	if (possible_uV > desired_max_uV) {
+		ret = -EINVAL;
+		goto out;
+	}
+	ret = possible_uV;
+
+out:
+	return ret;
+}
+
+static int regulator_balance_voltage(struct regulator_dev *rdev,
+				     suspend_state_t state)
+{
+	struct regulator_dev **c_rdevs;
+	struct regulator_dev *best_rdev;
+	struct coupling_desc *c_desc = &rdev->coupling_desc;
+	int n_coupled;
+	int i, best_delta, best_uV, ret = 1;
+
+	c_rdevs = c_desc->coupled_rdevs;
+	n_coupled = c_desc->n_coupled;
+
+	/*
+	 * if system is in a state other than PM_SUSPEND_ON, don't check
+	 * other coupled regulators
+	 */
+	if (state != PM_SUSPEND_ON)
+		n_coupled = 1;
+
+	/*
+	 * Find the best possible voltage change on each loop. Leave the loop
+	 * if there isn't any possible change.
+	 */
+	while (1) {
+		best_delta = 0;
+		best_uV = 0;
+		best_rdev = NULL;
+
+		/*
+		 * Find highest difference between optimal voltage
+		 * and current voltage.
+		 */
+		for (i = 0; i < n_coupled; i++) {
+			/*
+			 * optimal_uV is the best voltage that can be set for
+			 * i-th regulator at the moment without violating
+			 * max_spread constraint in order to balance
+			 * the coupled voltages.
+			 */
+			int optimal_uV, current_uV;
+
+			optimal_uV = regulator_get_optimal_voltage(c_rdevs[i]);
+			if (optimal_uV < 0) {
+				ret = optimal_uV;
+				goto out;
+			}
+
+			current_uV = _regulator_get_voltage(c_rdevs[i]);
+			if (current_uV < 0) {
+				ret = optimal_uV;
+				goto out;
+			}
+
+			if (abs(best_delta) < abs(optimal_uV - current_uV)) {
+				best_delta = optimal_uV - current_uV;
+				best_rdev = c_rdevs[i];
+				best_uV = optimal_uV;
+			}
+		}
+
+		/* Nothing to change, return successfully */
+		if (!best_rdev) {
+			ret = 0;
+			goto out;
+		}
+
+		/*
+		 * Lock just the supply regulators, as the regulator itself
+		 * is already locked by regulator_lock_coupled().
+		 */
+		if (best_rdev->supply)
+			regulator_lock_supply(best_rdev->supply->rdev);
+
+		ret = regulator_set_voltage_rdev(best_rdev, best_uV,
+						 best_uV, state);
+		if (best_rdev->supply)
+			regulator_unlock_supply(best_rdev->supply->rdev);
+
+		if (ret < 0)
+			goto out;
+	}
+
+out:
+	return ret;
+}
+
 /**
  * regulator_set_voltage - set regulator output voltage
  * @regulator: regulator source
-- 
2.7.4

[PATCH v5 5/5] regulator: core: Change voltage setting path

[Maciej Purski]

On Odroid XU3/4 and other Exynos5422 based boards there is a case, that
different devices on the board are supplied by different regulators
with non-fixed voltages. If one of these devices temporarily requires
higher voltage, there might occur a situation that the spread between
two devices' voltages is so high, that there is a risk of changing
'high' and 'low' states on the interconnection between devices powered
by those regulators.

Uncoupled regulators should be a special case of coupled regulators, so
they should share a common voltage setting path. When enabling,
disabling or setting voltage of a coupled regulator, all coupled
regulators should be locked. Regulator's supplies should be locked, when
setting voltage of a single regulator. Enabling a coupled regulator or
setting its voltage should not be possible if some of its coupled
regulators, has not been registered.

Add function for locking coupled regulators. Extract a new function
regulator_set_voltage_rdev() from regulator_set_voltage_unlocked(),
which is called when setting voltage of a single regulator.

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/regulator/core.c | 101 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 81 insertions(+), 20 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b24a987..dedf737 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -107,6 +107,9 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 				     int min_uV, int max_uV);
 static int regulator_balance_voltage(struct regulator_dev *rdev,
 				     suspend_state_t state);
+static int regulator_set_voltage_rdev(struct regulator_dev *rdev,
+				      int min_uV, int max_uV,
+				      suspend_state_t state);
 static struct regulator *create_regulator(struct regulator_dev *rdev,
 					  struct device *dev,
 					  const char *supply_name);
@@ -180,6 +183,36 @@ static void regulator_unlock_supply(struct regulator_dev *rdev)
 }
 
 /**
+ * regulator_lock_coupled - lock a group of coupled regulators
+ * @rdev:      regulator source
+ */
+static void regulator_lock_coupled(struct regulator_dev *rdev)
+{
+	struct coupling_desc *c_desc = &rdev->coupling_desc;
+	int n_coupled = c_desc->n_coupled;
+	int i;
+
+	for (i = 0; i < n_coupled; i++)
+		if (c_desc->coupled_rdevs[i])
+			mutex_lock_nested(&c_desc->coupled_rdevs[i]->mutex, i);
+}
+
+/**
+ * regulator_unlock_coupled - unlock a group of coupled regulators
+ * @rdev:      regulator source
+ */
+static void regulator_unlock_coupled(struct regulator_dev *rdev)
+{
+	struct coupling_desc *c_desc = &rdev->coupling_desc;
+	int n_coupled = c_desc->n_coupled;
+	int i;
+
+	for (i = 0; i < n_coupled; i++)
+		if (c_desc->coupled_rdevs[i])
+			mutex_unlock(&c_desc->coupled_rdevs[i]->mutex);
+}
+
+/**
  * 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
@@ -2199,6 +2232,11 @@ int regulator_enable(struct regulator *regulator)
 	struct regulator_dev *rdev = regulator->rdev;
 	int ret = 0;
 
+	if (rdev->coupling_desc.n_resolved != rdev->coupling_desc.n_coupled) {
+		rdev_err(rdev, "not all coupled regulators registered\n");
+		return -EPERM;
+	}
+
 	if (regulator->always_on)
 		return 0;
 
@@ -2208,9 +2246,10 @@ int regulator_enable(struct regulator *regulator)
 			return ret;
 	}
 
-	mutex_lock(&rdev->mutex);
+	regulator_lock_coupled(rdev);
 	ret = _regulator_enable(rdev);
-	mutex_unlock(&rdev->mutex);
+	regulator_balance_voltage(rdev, PM_SUSPEND_ON);
+	regulator_unlock_coupled(rdev);
 
 	if (ret != 0 && rdev->supply)
 		regulator_disable(rdev->supply);
@@ -2316,9 +2355,10 @@ int regulator_disable(struct regulator *regulator)
 	if (regulator->always_on)
 		return 0;
 
-	mutex_lock(&rdev->mutex);
+	regulator_lock_coupled(rdev);
 	ret = _regulator_disable(rdev);
-	mutex_unlock(&rdev->mutex);
+	regulator_balance_voltage(rdev, PM_SUSPEND_ON);
+	regulator_unlock_coupled(rdev);
 
 	if (ret == 0 && rdev->supply)
 		regulator_disable(rdev->supply);
@@ -2367,10 +2407,11 @@ int regulator_force_disable(struct regulator *regulator)
 	struct regulator_dev *rdev = regulator->rdev;
 	int ret;
 
-	mutex_lock(&rdev->mutex);
+	regulator_lock_coupled(rdev);
 	regulator->uA_load = 0;
 	ret = _regulator_force_disable(regulator->rdev);
-	mutex_unlock(&rdev->mutex);
+	regulator_balance_voltage(rdev, PM_SUSPEND_ON);
+	regulator_unlock_coupled(rdev);
 
 	if (rdev->supply)
 		while (rdev->open_count--)
@@ -2929,8 +2970,12 @@ 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 (rdev->coupling_desc.n_resolved != rdev->coupling_desc.n_coupled) {
+		rdev_err(rdev, "not all coupled regulators registered\n");
+		ret = -EPERM;
+		goto out;
+	}
 
 	/* If we're setting the same range as last time the change
 	 * should be a noop (some cpufreq implementations use the same
@@ -2974,6 +3019,27 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 	if (ret < 0)
 		goto out2;
 
+	/* for not coupled regulators this will just set the voltage */
+	ret = regulator_balance_voltage(rdev, state);
+	if (ret < 0)
+		goto out2;
+
+out:
+	return 0;
+out2:
+	voltage->min_uV = old_min_uV;
+	voltage->max_uV = old_max_uV;
+
+	return ret;
+}
+
+static int regulator_set_voltage_rdev(struct regulator_dev *rdev, int min_uV,
+				      int max_uV, suspend_state_t state)
+{
+	int best_supply_uV = 0;
+	int supply_change_uV = 0;
+	int ret;
+
 	if (rdev->supply &&
 	    regulator_ops_is_valid(rdev->supply->rdev,
 				   REGULATOR_CHANGE_VOLTAGE) &&
@@ -2985,13 +3051,13 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 		selector = regulator_map_voltage(rdev, min_uV, max_uV);
 		if (selector < 0) {
 			ret = selector;
-			goto out2;
+			goto out;
 		}
 
 		best_supply_uV = _regulator_list_voltage(rdev, selector, 0);
 		if (best_supply_uV < 0) {
 			ret = best_supply_uV;
-			goto out2;
+			goto out;
 		}
 
 		best_supply_uV += rdev->desc->min_dropout_uV;
@@ -2999,7 +3065,7 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 		current_supply_uV = _regulator_get_voltage(rdev->supply->rdev);
 		if (current_supply_uV < 0) {
 			ret = current_supply_uV;
-			goto out2;
+			goto out;
 		}
 
 		supply_change_uV = best_supply_uV - current_supply_uV;
@@ -3011,7 +3077,7 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 		if (ret) {
 			dev_err(&rdev->dev, "Failed to increase supply voltage: %d\n",
 					ret);
-			goto out2;
+			goto out;
 		}
 	}
 
@@ -3021,7 +3087,7 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 		ret = _regulator_do_set_suspend_voltage(rdev, min_uV,
 							max_uV, state);
 	if (ret < 0)
-		goto out2;
+		goto out;
 
 	if (supply_change_uV < 0) {
 		ret = regulator_set_voltage_unlocked(rdev->supply,
@@ -3035,11 +3101,6 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 
 out:
 	return ret;
-out2:
-	voltage->min_uV = old_min_uV;
-	voltage->max_uV = old_max_uV;
-
-	return ret;
 }
 
 static int regulator_get_optimal_voltage(struct regulator_dev *rdev)
@@ -3254,12 +3315,12 @@ int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
 {
 	int ret = 0;
 
-	regulator_lock_supply(regulator->rdev);
+	regulator_lock_coupled(regulator->rdev);
 
 	ret = regulator_set_voltage_unlocked(regulator, min_uV, max_uV,
 					     PM_SUSPEND_ON);
 
-	regulator_unlock_supply(regulator->rdev);
+	regulator_unlock_coupled(regulator->rdev);
 
 	return ret;
 }
-- 
2.7.4

Re: [PATCH v5 0/5] Add coupled regulators mechanism

[Fabio Estevam]

Hi Maciej,

On Fri, Mar 2, 2018 at 5:42 AM, Maciej Purski <m.purski@samsung.com> wrote:
> Hi all,
>
> this patchset adds a new mechanism to the framework - regulators' coupling.
>
> On Odroid XU3/4 and other Exynos5422 based boards there is a case, that
> different devices on the board are supplied by different regulators
> with non-fixed voltages. If one of these devices temporarily requires
> higher voltage, there might occur a situation that the spread between
> devices' voltages is so high, that there is a risk of changing
> 'high' and 'low' states on the interconnection between devices powered
> by those regulators.
>
> Algorithmicaly the problem was solved by:
> Inderpal Singh <inderpal.s@samsung.com>
> Doug Anderson <dianders@chromium.org>
>
> The discussion on that subject can be found here:
> https://lkml.org/lkml/2014/4/29/28
>
> Therefore this patchset is an attempt to apply the idea to regulators core
> as concluded in the discussion by Mark Brown and Doug Anderson.
>
> This feature is required to enable support for generic CPUfreq
> and devfreq drivers for the mentioned boards.
>
> Note on the locking model:
> When balancing voltage of a group of coupled regulators, we lock all
> of them for the whole operation. When voltage of an individual regulator
> is about to change, its suppliers are additionally locked.
>
> The current assumption is that an uncoupled regulator is a special case
> of a coupled one, so they should share a common voltage setting path.

This series breaks has reached linux-next 20180305 and it breaks
booting on imx6:

[    0.269646] imx-pgc-pd imx-pgc-power-domain.0: Linked as a consumer
to 20dc000.gpc
[    0.270348]
[    0.270363] ============================================
[    0.270373] WARNING: possible recursive locking detected
[    0.270385] 4.16.0-rc3-next-20180305 #156 Not tainted
[    0.270397] --------------------------------------------
[    0.270408] swapper/0/1 is trying to acquire lock:
[    0.270419]  (&rdev->mutex){+.+.}, at: [<c04d80c4>]
regulator_lock_supply+0x24/0x44
[    0.270460]
[    0.270460] but task is already holding lock:
[    0.270471]  (&rdev->mutex){+.+.}, at: [<c04dc6e4>]
regulator_enable+0x78/0x298
[    0.270502]
[    0.270502] other info that might help us debug this:
[    0.270513]  Possible unsafe locking scenario:
[    0.270513]
[    0.270523]        CPU0
[    0.270532]        ----
[    0.270540]   lock(&rdev->mutex);
[    0.270555]   lock(&rdev->mutex);
[    0.270572]
[    0.270572]  *** DEADLOCK ***
[    0.270572]
[    0.270585]  May be due to missing lock nesting notation
[    0.270585]
[    0.270598] 3 locks held by swapper/0/1:
[    0.270606]  #0:  (&dev->mutex){....}, at: [<c057f368>]
__driver_attach+0x58/0xcc
[    0.270642]  #1:  (&dev->mutex){....}, at: [<c057f378>]
__driver_attach+0x68/0xcc
[    0.270672]  #2:  (&rdev->mutex){+.+.}, at: [<c04dc6e4>]
regulator_enable+0x78/0x298
[    0.270701]
[    0.270701] stack backtrace:
[    0.270719] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
4.16.0-rc3-next-20180305 #156
[    0.270731] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[    0.270741] Backtrace:
[    0.270767] [<c010d154>] (dump_backtrace) from [<c010d414>]
(show_stack+0x18/0x1c)
[    0.270783]  r7:00000000 r6:60000093 r5:00000000 r4:c1078360
[    0.270805] [<c010d3fc>] (show_stack) from [<c0a4724c>]
(dump_stack+0xb4/0xe8)
[    0.270830] [<c0a47198>] (dump_stack) from [<c0174f4c>]
(__lock_acquire+0x13a4/0x199c)
[    0.270848]  r9:c17d75fc r8:c11ae35c r7:c17c73b0 r6:ec060000
r5:ec060570 r4:c11ae35c
[    0.270867] [<c0173ba8>] (__lock_acquire) from [<c0175d68>]
(lock_acquire+0x70/0x90)
[    0.270883]  r10:00000000 r9:ec195800 r8:00000001 r7:00000001
r6:60000013 r5:00000000
[    0.270893]  r4:ffffe000
[    0.270914] [<c0175cf8>] (lock_acquire) from [<c0a60760>]
(__mutex_lock+0x64/0x978)
[    0.270929]  r8:00000001 r7:00000001 r6:00000000 r5:00000000 r4:ec00b89c
[    0.270947] [<c0a606fc>] (__mutex_lock) from [<c0a61098>]
(mutex_lock_nested+0x24/0x2c)
[    0.270963]  r10:00000000 r9:ec195800 r8:00000001 r7:00000001
r6:ec195800 r5:00000000
[    0.270975]  r4:ec00b800
[    0.270992] [<c0a61074>] (mutex_lock_nested) from [<c04d80c4>]
(regulator_lock_supply+0x24/0x44)
[    0.271013] [<c04d80a0>] (regulator_lock_supply) from [<c04da9b0>]
(regulator_balance_voltage+0x298/0x584)
[    0.271024]  r5:000b1008 r4:ec195800
[    0.271042] [<c04da718>] (regulator_balance_voltage) from
[<c04dc748>] (regulator_enable+0xdc/0x298)
[    0.271059]  r10:00000000 r9:00000000 r8:00000001 r7:00000001
r6:ec195800 r5:ec195824
[    0.271069]  r4:00000001
[    0.271086] [<c04dc66c>] (regulator_enable) from [<c04d6dd8>]
(imx6_pm_domain_power_on+0x30/0x1b4)
[    0.271102]  r9:00000000 r8:c102d114 r7:c1008908 r6:c102d114
r5:ec3d7a00 r4:ec3d7a00
[    0.271119] [<c04d6da8>] (imx6_pm_domain_power_on) from
[<c04d6d18>] (imx_pgc_power_domain_probe+0x64/0xf4)
[    0.271135]  r8:c102d114 r7:ec49c810 r6:c102d114 r5:ec49c800 r4:ec3d7a00
[    0.271153] [<c04d6cb4>] (imx_pgc_power_domain_probe) from
[<c0580ef0>] (platform_drv_probe+0x54/0xb8)
[    0.271168]  r7:fffffdfb r6:c102d114 r5:ec49c810 r4:ec49c810
[    0.271189] [<c0580e9c>] (platform_drv_probe) from [<c057f288>]
(driver_probe_device+0x2b0/0x338)
[    0.271204]  r7:c17e234c r6:00000000 r5:c17e2348 r4:ec49c810
[    0.271222] [<c057efd8>] (driver_probe_device) from [<c057f3d8>]
(__driver_attach+0xc8/0xcc)
[    0.271237]  r10:c0f00630 r9:00000000 r8:c1008908 r7:c1008908
r6:ec49c844 r5:c102d114
[    0.271248]  r4:ec49c810 r3:00000000
[    0.271267] [<c057f310>] (__driver_attach) from [<c057d348>]
(bus_for_each_dev+0x6c/0xbc)
[    0.271280]  r7:c1008908 r6:c057f310 r5:c102d114 r4:00000000
[    0.271298] [<c057d2dc>] (bus_for_each_dev) from [<c057eb64>]
(driver_attach+0x20/0x28)
[    0.271314]  r7:00000000 r6:c103b0a8 r5:ec479b00 r4:c102d114
[    0.271332] [<c057eb44>] (driver_attach) from [<c057e588>]
(bus_add_driver+0x18c/0x214)
[    0.271351] [<c057e3fc>] (bus_add_driver) from [<c057fdbc>]
(driver_register+0x80/0x100)
[    0.271366]  r7:c0e125e4 r6:c0f63850 r5:c0f41c18 r4:c102d114
[    0.271383] [<c057fd3c>] (driver_register) from [<c0580e44>]
(__platform_driver_register+0x38/0x4c)
[    0.271396]  r5:c0f41c18 r4:ffffe000
[    0.271418] [<c0580e0c>] (__platform_driver_register) from
[<c0f41c30>] (imx_pgc_power_domain_driver_init+0x18/0x20)
[    0.271438] [<c0f41c18>] (imx_pgc_power_domain_driver_init) from
[<c0102764>] (do_one_initcall+0x50/0x1a0)
[    0.271464] [<c0102714>] (do_one_initcall) from [<c0f00f04>]
(kernel_init_freeable+0x118/0x1d0)
[    0.271481]  r9:c0f63858 r8:000000f2 r7:c0e125e4 r6:c0f63850
r5:c1079f80 r4:c0f77ab4
[    0.271508] [<c0f00dec>] (kernel_init_freeable) from [<c0a5c6f0>]
(kernel_init+0x10/0x118)
[    0.271524]  r10:00000000 r9:00000000 r8:00000000 r7:00000000
r6:00000000 r5:c0a5c6e0
[    0.271533]  r4:00000000
[    0.271552] [<c0a5c6e0>] (kernel_init) from [<c01010b4>]
(ret_from_fork+0x14/0x20)
[    0.271564] Exception stack(0xec069fb0 to 0xec069ff8)
[    0.271579] 9fa0:                                     00000000
00000000 00000000 00000000
[    0.271594] 9fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[    0.271609] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    0.271621]  r5:c0a5c6e0 r4:00000000
[    0.271723] imx-pgc-pd imx-pgc-power-domain.1: Linked as a consumer
to 20dc000.gpc
[    0.273566] 2020000.serial: ttymxc0 at MMIO 0x2020000 (irq = 26,
base_baud = 5000000) is a IMX
[    1.660948] console [ttymxc0] enabled
[    1.666242] 21ec000.serial: ttymxc2 at MMIO 0x21ec000 (irq = 65,
base_baud = 5000000) is a IMX
[    1.683484] etnaviv etnaviv: bound 130000.gpu (ops gpu_ops)
[    1.689464] etnaviv etnaviv: bound 134000.gpu (ops gpu_ops)

If I revert this series I am able to boot again.

Re: [PATCH v5 0/5] Add coupled regulators mechanism

[Maciej Purski]

Hi Fabio,
thank you for your response.
Could you tell me on which board precisely (on which DTS) has this issue occurred?
Also, I don't understand, did you boot successfully and you got only a lockdep 
warning or you encountered an actual deadlock?

Best regards,
Maciej Purski


On 03/05/2018 01:34 PM, Fabio Estevam wrote:
> Hi Maciej,
> 
> On Fri, Mar 2, 2018 at 5:42 AM, Maciej Purski <m.purski@samsung.com> wrote:
>> Hi all,
>>
>> this patchset adds a new mechanism to the framework - regulators' coupling.
>>
>> On Odroid XU3/4 and other Exynos5422 based boards there is a case, that
>> different devices on the board are supplied by different regulators
>> with non-fixed voltages. If one of these devices temporarily requires
>> higher voltage, there might occur a situation that the spread between
>> devices' voltages is so high, that there is a risk of changing
>> 'high' and 'low' states on the interconnection between devices powered
>> by those regulators.
>>
>> Algorithmicaly the problem was solved by:
>> Inderpal Singh <inderpal.s@samsung.com>
>> Doug Anderson <dianders@chromium.org>
>>
>> The discussion on that subject can be found here:
>> https://lkml.org/lkml/2014/4/29/28
>>
>> Therefore this patchset is an attempt to apply the idea to regulators core
>> as concluded in the discussion by Mark Brown and Doug Anderson.
>>
>> This feature is required to enable support for generic CPUfreq
>> and devfreq drivers for the mentioned boards.
>>
>> Note on the locking model:
>> When balancing voltage of a group of coupled regulators, we lock all
>> of them for the whole operation. When voltage of an individual regulator
>> is about to change, its suppliers are additionally locked.
>>
>> The current assumption is that an uncoupled regulator is a special case
>> of a coupled one, so they should share a common voltage setting path.
> 
> This series breaks has reached linux-next 20180305 and it breaks
> booting on imx6:
> 
> [    0.269646] imx-pgc-pd imx-pgc-power-domain.0: Linked as a consumer
> to 20dc000.gpc
> [    0.270348]
> [    0.270363] ============================================
> [    0.270373] WARNING: possible recursive locking detected
> [    0.270385] 4.16.0-rc3-next-20180305 #156 Not tainted
> [    0.270397] --------------------------------------------
> [    0.270408] swapper/0/1 is trying to acquire lock:
> [    0.270419]  (&rdev->mutex){+.+.}, at: [<c04d80c4>]
> regulator_lock_supply+0x24/0x44
> [    0.270460]
> [    0.270460] but task is already holding lock:
> [    0.270471]  (&rdev->mutex){+.+.}, at: [<c04dc6e4>]
> regulator_enable+0x78/0x298
> [    0.270502]
> [    0.270502] other info that might help us debug this:
> [    0.270513]  Possible unsafe locking scenario:
> [    0.270513]
> [    0.270523]        CPU0
> [    0.270532]        ----
> [    0.270540]   lock(&rdev->mutex);
> [    0.270555]   lock(&rdev->mutex);
> [    0.270572]
> [    0.270572]  *** DEADLOCK ***
> [    0.270572]
> [    0.270585]  May be due to missing lock nesting notation
> [    0.270585]
> [    0.270598] 3 locks held by swapper/0/1:
> [    0.270606]  #0:  (&dev->mutex){....}, at: [<c057f368>]
> __driver_attach+0x58/0xcc
> [    0.270642]  #1:  (&dev->mutex){....}, at: [<c057f378>]
> __driver_attach+0x68/0xcc
> [    0.270672]  #2:  (&rdev->mutex){+.+.}, at: [<c04dc6e4>]
> regulator_enable+0x78/0x298
> [    0.270701]
> [    0.270701] stack backtrace:
> [    0.270719] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 4.16.0-rc3-next-20180305 #156
> [    0.270731] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [    0.270741] Backtrace:
> [    0.270767] [<c010d154>] (dump_backtrace) from [<c010d414>]
> (show_stack+0x18/0x1c)
> [    0.270783]  r7:00000000 r6:60000093 r5:00000000 r4:c1078360
> [    0.270805] [<c010d3fc>] (show_stack) from [<c0a4724c>]
> (dump_stack+0xb4/0xe8)
> [    0.270830] [<c0a47198>] (dump_stack) from [<c0174f4c>]
> (__lock_acquire+0x13a4/0x199c)
> [    0.270848]  r9:c17d75fc r8:c11ae35c r7:c17c73b0 r6:ec060000
> r5:ec060570 r4:c11ae35c
> [    0.270867] [<c0173ba8>] (__lock_acquire) from [<c0175d68>]
> (lock_acquire+0x70/0x90)
> [    0.270883]  r10:00000000 r9:ec195800 r8:00000001 r7:00000001
> r6:60000013 r5:00000000
> [    0.270893]  r4:ffffe000
> [    0.270914] [<c0175cf8>] (lock_acquire) from [<c0a60760>]
> (__mutex_lock+0x64/0x978)
> [    0.270929]  r8:00000001 r7:00000001 r6:00000000 r5:00000000 r4:ec00b89c
> [    0.270947] [<c0a606fc>] (__mutex_lock) from [<c0a61098>]
> (mutex_lock_nested+0x24/0x2c)
> [    0.270963]  r10:00000000 r9:ec195800 r8:00000001 r7:00000001
> r6:ec195800 r5:00000000
> [    0.270975]  r4:ec00b800
> [    0.270992] [<c0a61074>] (mutex_lock_nested) from [<c04d80c4>]
> (regulator_lock_supply+0x24/0x44)
> [    0.271013] [<c04d80a0>] (regulator_lock_supply) from [<c04da9b0>]
> (regulator_balance_voltage+0x298/0x584)
> [    0.271024]  r5:000b1008 r4:ec195800
> [    0.271042] [<c04da718>] (regulator_balance_voltage) from
> [<c04dc748>] (regulator_enable+0xdc/0x298)
> [    0.271059]  r10:00000000 r9:00000000 r8:00000001 r7:00000001
> r6:ec195800 r5:ec195824
> [    0.271069]  r4:00000001
> [    0.271086] [<c04dc66c>] (regulator_enable) from [<c04d6dd8>]
> (imx6_pm_domain_power_on+0x30/0x1b4)
> [    0.271102]  r9:00000000 r8:c102d114 r7:c1008908 r6:c102d114
> r5:ec3d7a00 r4:ec3d7a00
> [    0.271119] [<c04d6da8>] (imx6_pm_domain_power_on) from
> [<c04d6d18>] (imx_pgc_power_domain_probe+0x64/0xf4)
> [    0.271135]  r8:c102d114 r7:ec49c810 r6:c102d114 r5:ec49c800 r4:ec3d7a00
> [    0.271153] [<c04d6cb4>] (imx_pgc_power_domain_probe) from
> [<c0580ef0>] (platform_drv_probe+0x54/0xb8)
> [    0.271168]  r7:fffffdfb r6:c102d114 r5:ec49c810 r4:ec49c810
> [    0.271189] [<c0580e9c>] (platform_drv_probe) from [<c057f288>]
> (driver_probe_device+0x2b0/0x338)
> [    0.271204]  r7:c17e234c r6:00000000 r5:c17e2348 r4:ec49c810
> [    0.271222] [<c057efd8>] (driver_probe_device) from [<c057f3d8>]
> (__driver_attach+0xc8/0xcc)
> [    0.271237]  r10:c0f00630 r9:00000000 r8:c1008908 r7:c1008908
> r6:ec49c844 r5:c102d114
> [    0.271248]  r4:ec49c810 r3:00000000
> [    0.271267] [<c057f310>] (__driver_attach) from [<c057d348>]
> (bus_for_each_dev+0x6c/0xbc)
> [    0.271280]  r7:c1008908 r6:c057f310 r5:c102d114 r4:00000000
> [    0.271298] [<c057d2dc>] (bus_for_each_dev) from [<c057eb64>]
> (driver_attach+0x20/0x28)
> [    0.271314]  r7:00000000 r6:c103b0a8 r5:ec479b00 r4:c102d114
> [    0.271332] [<c057eb44>] (driver_attach) from [<c057e588>]
> (bus_add_driver+0x18c/0x214)
> [    0.271351] [<c057e3fc>] (bus_add_driver) from [<c057fdbc>]
> (driver_register+0x80/0x100)
> [    0.271366]  r7:c0e125e4 r6:c0f63850 r5:c0f41c18 r4:c102d114
> [    0.271383] [<c057fd3c>] (driver_register) from [<c0580e44>]
> (__platform_driver_register+0x38/0x4c)
> [    0.271396]  r5:c0f41c18 r4:ffffe000
> [    0.271418] [<c0580e0c>] (__platform_driver_register) from
> [<c0f41c30>] (imx_pgc_power_domain_driver_init+0x18/0x20)
> [    0.271438] [<c0f41c18>] (imx_pgc_power_domain_driver_init) from
> [<c0102764>] (do_one_initcall+0x50/0x1a0)
> [    0.271464] [<c0102714>] (do_one_initcall) from [<c0f00f04>]
> (kernel_init_freeable+0x118/0x1d0)
> [    0.271481]  r9:c0f63858 r8:000000f2 r7:c0e125e4 r6:c0f63850
> r5:c1079f80 r4:c0f77ab4
> [    0.271508] [<c0f00dec>] (kernel_init_freeable) from [<c0a5c6f0>]
> (kernel_init+0x10/0x118)
> [    0.271524]  r10:00000000 r9:00000000 r8:00000000 r7:00000000
> r6:00000000 r5:c0a5c6e0
> [    0.271533]  r4:00000000
> [    0.271552] [<c0a5c6e0>] (kernel_init) from [<c01010b4>]
> (ret_from_fork+0x14/0x20)
> [    0.271564] Exception stack(0xec069fb0 to 0xec069ff8)
> [    0.271579] 9fa0:                                     00000000
> 00000000 00000000 00000000
> [    0.271594] 9fc0: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [    0.271609] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [    0.271621]  r5:c0a5c6e0 r4:00000000
> [    0.271723] imx-pgc-pd imx-pgc-power-domain.1: Linked as a consumer
> to 20dc000.gpc
> [    0.273566] 2020000.serial: ttymxc0 at MMIO 0x2020000 (irq = 26,
> base_baud = 5000000) is a IMX
> [    1.660948] console [ttymxc0] enabled
> [    1.666242] 21ec000.serial: ttymxc2 at MMIO 0x21ec000 (irq = 65,
> base_baud = 5000000) is a IMX
> [    1.683484] etnaviv etnaviv: bound 130000.gpu (ops gpu_ops)
> [    1.689464] etnaviv etnaviv: bound 134000.gpu (ops gpu_ops)
> 
> If I revert this series I am able to boot again.
> 
> 
> 

Re: [PATCH v5 0/5] Add coupled regulators mechanism

[Fabio Estevam]

Hi Maciej,

On Mon, Mar 5, 2018 at 11:40 AM, Maciej Purski <m.purski@samsung.com> wrote:
> Hi Fabio,
> thank you for your response.
> Could you tell me on which board precisely (on which DTS) has this issue
> occurred?

I tested it on a imx6dl-wandboard.dtb.

kernelci.org also shows other imx6 boards that cannot boot with
today's linux-next.

> Also, I don't understand, did you boot successfully and you got only a
> lockdep warning or you encountered an actual deadlock?

No, with this series I am not able to boot till the Linux prompt.

Thanks

Re: [PATCH v5 0/5] Add coupled regulators mechanism

[Fabio Estevam]

Hi Maciej,

On Mon, Mar 5, 2018 at 12:57 PM, Fabio Estevam <festevam@gmail.com> wrote:

> kernelci.org also shows other imx6 boards that cannot boot with
> today's linux-next.

Here are the completes logs in case they help:

https://storage.kernelci.org/next/master/next-20180305/arm/imx_v6_v7_defconfig/lab-free-electrons/boot-imx6q-nitrogen6x.txt

https://storage.kernelci.org/next/master/next-20180305/arm/imx_v6_v7_defconfig/lab-baylibre-seattle/boot-imx6dl-wandboard_solo.txt