[RFC PATCH v1 0/6] Introduce machine-specific regulators coupling API

[RFC PATCH v1 0/6] Introduce machine-specific regulators coupling API

[Dmitry Osipenko]

Hello,

I was looking into how to properly implement regulators coupling for
NVIDIA Tegra SoC's and ended up with this patchset that introduces
machine-specific regulators coupling. Upstream kernel now has support
for a simple variants of regulators coupling in a form of limiting
maximum voltage spreading between two regulators, but that's not enough
for the case of Tegra SoC's. It's a bit difficult to support universally
all possible coupling restrictions in a form of device-tree description,
so here comes the machine-specific coupling API which allow platforms
to customize coupling algorithms.

Dmitry Osipenko (6):
  regulator: core: Introduce API for machine-specific regulators
    coupling
  regulator: core: Parse max-spread value per regulator couple
  regulator: core: Expose some of core functions
  regulator: core Bump MAX_COUPLED to 3
  soc/tegra: regulators: Add regulators coupler for Tegra20
  soc/tegra: regulators: Add regulators coupler for Tegra30

 drivers/regulator/core.c               |  89 +++++---
 drivers/regulator/of_regulator.c       |  49 ++--
 drivers/soc/tegra/Kconfig              |  12 +
 drivers/soc/tegra/Makefile             |   2 +
 drivers/soc/tegra/regulators-tegra20.c | 304 +++++++++++++++++++++++++
 drivers/soc/tegra/regulators-tegra30.c | 256 +++++++++++++++++++++
 include/linux/regulator/driver.h       |  14 +-
 include/linux/regulator/machine.h      |  22 +-
 8 files changed, 694 insertions(+), 54 deletions(-)
 create mode 100644 drivers/soc/tegra/regulators-tegra20.c
 create mode 100644 drivers/soc/tegra/regulators-tegra30.c

-- 
2.21.0

[RFC PATCH v1 1/6] regulator: core: Introduce API for machine-specific regulators coupling

[Dmitry Osipenko]

Right now regulator core supports only one type of regulators coupling,
the "voltage max-spread" which keeps voltages of coupled regulators in a
given range. A more sophisticated coupling may be required in practice,
one example is the NVIDIA Tegra SoC's which besides the max-spreading
have other restrictions that must be adhered. Introduce API that allow
platforms to provide their own custom coupling algorithms.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/regulator/core.c          | 18 ++++++++++++++++++
 include/linux/regulator/machine.h | 19 +++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 186a37675b50..a98af47e0feb 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -59,6 +59,8 @@ static bool has_full_constraints;
 
 static struct dentry *debugfs_root;
 
+static struct regulators_coupler *machine_regulators_coupler;
+
 /*
  * struct regulator_map
  *
@@ -3596,6 +3598,12 @@ static int regulator_balance_voltage(struct regulator_dev *rdev,
 		return -EPERM;
 	}
 
+	if (n_coupled > 1 &&
+	    machine_regulators_coupler &&
+	    machine_regulators_coupler->balance_voltage)
+		return machine_regulators_coupler->balance_voltage(
+				machine_regulators_coupler, rdev, state);
+
 	for (i = 0; i < n_coupled; i++)
 		c_rdev_done[i] = false;
 
@@ -4706,6 +4714,16 @@ static int regulator_register_resolve_supply(struct device *dev, void *data)
 	return 0;
 }
 
+int regulators_coupler_register(struct regulators_coupler *coupler)
+{
+	if (WARN_ON(machine_regulators_coupler))
+		return -EBUSY;
+
+	machine_regulators_coupler = coupler;
+
+	return 0;
+}
+
 static void regulator_resolve_coupling(struct regulator_dev *rdev)
 {
 	struct coupling_desc *c_desc = &rdev->coupling_desc;
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index 1d34a70ffda2..f15c8c5f7f6c 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -19,6 +19,7 @@
 #include <linux/suspend.h>
 
 struct regulator;
+struct regulator_dev;
 
 /*
  * Regulator operation constraint flags. These flags are used to enable
@@ -265,4 +266,22 @@ static inline int regulator_suspend_finish(void)
 	return 0;
 }
 
+/**
+ * struct regulators_coupler - machine-specific regulators coupler
+ *
+ * A custom regulators coupler allows platform to customize coupling
+ * algorithm.
+ *
+ * @balance_voltage: Callback invoked when voltage of a coupled regulator is
+ *                   changing. The callee should perform voltage balancing
+ *                   and change voltage of the coupled regulators.
+ */
+struct regulators_coupler {
+	int (*balance_voltage)(struct regulators_coupler *coupler,
+			       struct regulator_dev *rdev,
+			       suspend_state_t state);
+};
+
+int regulators_coupler_register(struct regulators_coupler *coupler);
+
 #endif
-- 
2.21.0

Re: [RFC PATCH v1 1/6] regulator: core: Introduce API for machine-specific regulators coupling

[Mark Brown]

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

On Sun, Apr 14, 2019 at 08:59:34PM +0300, Dmitry Osipenko wrote:
> Right now regulator core supports only one type of regulators coupling,
> the "voltage max-spread" which keeps voltages of coupled regulators in a
> given range. A more sophisticated coupling may be required in practice,
> one example is the NVIDIA Tegra SoC's which besides the max-spreading
> have other restrictions that must be adhered. Introduce API that allow
> platforms to provide their own custom coupling algorithms.

This is really concerning since it's jumping straight to open coding the
algorithm in platform specific code which isn't great, especially since
that platform specific code is now going to have to handle all possible
board specific restrictions that might be found on that platform.  Why
is it not possible to express the rules that exist in a more general
fashion which can be encoded in drivers?  I'm not thrilled about later
patches that export core functionality for platform specific use either.

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

Re: [RFC PATCH v1 1/6] regulator: core: Introduce API for machine-specific regulators coupling

[Dmitry Osipenko]

08.05.2019 10:55, Mark Brown пишет:
> On Sun, Apr 14, 2019 at 08:59:34PM +0300, Dmitry Osipenko wrote:
>> Right now regulator core supports only one type of regulators coupling,
>> the "voltage max-spread" which keeps voltages of coupled regulators in a
>> given range. A more sophisticated coupling may be required in practice,
>> one example is the NVIDIA Tegra SoC's which besides the max-spreading
>> have other restrictions that must be adhered. Introduce API that allow
>> platforms to provide their own custom coupling algorithms.
> 
> This is really concerning since it's jumping straight to open coding the
> algorithm in platform specific code which isn't great, especially since
> that platform specific code is now going to have to handle all possible
> board specific restrictions that might be found on that platform.  Why
> is it not possible to express the rules that exist in a more general
> fashion which can be encoded in drivers?  I'm not thrilled about later
> patches that export core functionality for platform specific use either.
> 

Indeed, it's absolutely not the part of the idea that platform specific
code will be handling board specifics as well. I just wanted to KISS for
the first draft and will change that for a more generic solution in the
next revision.

[RFC PATCH v1 2/6] regulator: core: Parse max-spread value per regulator couple

[Dmitry Osipenko]

Parse and validate max-spread value per regulator couple. Now regulator
core can take multiple regulator couples, although balancing is still
limited to a single couple.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/regulator/core.c          | 13 ++++----
 drivers/regulator/of_regulator.c  | 49 +++++++++++++++++++++----------
 include/linux/regulator/machine.h |  3 +-
 3 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index a98af47e0feb..cdb25bbee631 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3440,7 +3440,7 @@ 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;
 	struct regulation_constraints *constraints = rdev->constraints;
-	int max_spread = constraints->max_spread;
+	int max_spread = constraints->max_spread[0];
 	int desired_min_uV = 0, desired_max_uV = INT_MAX;
 	int max_current_uV = 0, min_current_uV = INT_MAX;
 	int highest_min_uV = 0, target_uV, possible_uV;
@@ -3604,6 +3604,12 @@ static int regulator_balance_voltage(struct regulator_dev *rdev,
 		return machine_regulators_coupler->balance_voltage(
 				machine_regulators_coupler, rdev, state);
 
+	if (n_coupled > 2) {
+		rdev_err(rdev,
+			 "Voltage balancing for multiple regulator couples is unimplemented\n");
+		return -EPERM;
+	}
+
 	for (i = 0; i < n_coupled; i++)
 		c_rdev_done[i] = false;
 
@@ -4821,11 +4827,6 @@ static int regulator_init_coupling(struct regulator_dev *rdev)
 		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;
 
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 6dca0ba044d8..ccca09de92d6 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -34,8 +34,14 @@ static void of_get_regulation_constraints(struct device_node *np,
 	struct device_node *suspend_np;
 	unsigned int mode;
 	int ret, i, len;
+	int n_phandles;
+	u32 pvals[MAX_COUPLED];
 	u32 pval;
 
+	n_phandles = of_count_phandle_with_args(np, "regulator-coupled-with",
+						NULL);
+	n_phandles = max(n_phandles, 0);
+
 	constraints->name = of_get_property(np, "regulator-name", NULL);
 
 	if (!of_property_read_u32(np, "regulator-min-microvolt", &pval))
@@ -167,9 +173,12 @@ 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;
+	if (!of_property_read_u32_array(np, "regulator-coupled-max-spread",
+					pvals, n_phandles)) {
+
+		for (i = 0; i < n_phandles; i++)
+			constraints->max_spread[i] = pvals[i];
+	}
 
 	if (!of_property_read_u32(np, "regulator-max-step-microvolt",
 				  &pval))
@@ -477,7 +486,8 @@ int of_get_n_coupled(struct regulator_dev *rdev)
 
 /* 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)
+				  struct device_node *to_find,
+				  int *index)
 {
 	int n_phandles, i;
 	bool found = false;
@@ -499,8 +509,10 @@ static bool of_coupling_find_node(struct device_node *src,
 
 		of_node_put(tmp);
 
-		if (found)
+		if (found) {
+			*index = i;
 			break;
+		}
 	}
 
 	return found;
@@ -521,22 +533,28 @@ static bool of_coupling_find_node(struct device_node *src,
  */
 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 index;
 	int i;
 	bool ret = true;
 
-	if (max_spread <= 0) {
-		dev_err(&rdev->dev, "max_spread value invalid\n");
+	if (n_phandles >= MAX_COUPLED) {
+		dev_err(&rdev->dev, "please bump MAX_COUPLED number\n");
 		return false;
 	}
 
 	/* iterate over rdev's phandles */
 	for (i = 0; i < n_phandles; i++) {
+		int max_spread = rdev->constraints->max_spread[i];
 		int c_max_spread, c_n_phandles;
 
+		if (max_spread <= 0) {
+			dev_err(&rdev->dev, "max_spread value invalid\n");
+			return false;
+		}
+
 		c_node = of_parse_phandle(node,
 					  "regulator-coupled-with", i);
 
@@ -553,22 +571,23 @@ bool of_check_coupling_data(struct regulator_dev *rdev)
 			goto clean;
 		}
 
-		if (of_property_read_u32(c_node, "regulator-coupled-max-spread",
-					 &c_max_spread)) {
+		if (!of_coupling_find_node(c_node, node, &index)) {
+			dev_err(&rdev->dev, "missing 2-way linking for coupled regulators\n");
 			ret = false;
 			goto clean;
 		}
 
-		if (c_max_spread != max_spread) {
-			dev_err(&rdev->dev,
-				"coupled regulators max_spread mismatch\n");
+		if (of_property_read_u32_index(c_node, "regulator-coupled-max-spread",
+					       index, &c_max_spread)) {
 			ret = false;
 			goto clean;
 		}
 
-		if (!of_coupling_find_node(c_node, node)) {
-			dev_err(&rdev->dev, "missing 2-way linking for coupled regulators\n");
+		if (c_max_spread != max_spread) {
+			dev_err(&rdev->dev,
+				"coupled regulators max_spread mismatch\n");
 			ret = false;
+			goto clean;
 		}
 
 clean:
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index f15c8c5f7f6c..0ade1247d3f6 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -15,6 +15,7 @@
 #ifndef __LINUX_REGULATOR_MACHINE_H_
 #define __LINUX_REGULATOR_MACHINE_H_
 
+#include <linux/regulator/driver.h>
 #include <linux/regulator/consumer.h>
 #include <linux/suspend.h>
 
@@ -157,7 +158,7 @@ struct regulation_constraints {
 	int system_load;
 
 	/* used for coupled regulators */
-	int max_spread;
+	int max_spread[MAX_COUPLED];
 
 	/* used for changing voltage in steps */
 	int max_uV_step;
-- 
2.21.0

[RFC PATCH v1 3/6] regulator: core: Expose some of core functions

[Dmitry Osipenko]

Expose some of internal functions that are required for implementing
machine-specific regulators coupling for NVIDIA Tegra.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/regulator/core.c         | 58 +++++++++++++++-----------------
 include/linux/regulator/driver.h | 12 +++++++
 2 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index cdb25bbee631..fa2f4e9e8bcd 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -100,7 +100,6 @@ struct regulator_supply_alias {
 
 static int _regulator_is_enabled(struct regulator_dev *rdev);
 static int _regulator_disable(struct regulator *regulator);
-static int _regulator_get_voltage(struct regulator_dev *rdev);
 static int _regulator_get_current_limit(struct regulator_dev *rdev);
 static unsigned int _regulator_get_mode(struct regulator_dev *rdev);
 static int _notifier_call_chain(struct regulator_dev *rdev,
@@ -109,15 +108,12 @@ 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);
 static void _regulator_put(struct regulator *regulator);
 
-static const char *rdev_get_name(struct regulator_dev *rdev)
+const char *rdev_get_name(struct regulator_dev *rdev)
 {
 	if (rdev->constraints && rdev->constraints->name)
 		return rdev->constraints->name;
@@ -431,8 +427,8 @@ static struct device_node *of_get_regulator(struct device *dev, const char *supp
 }
 
 /* Platform voltage constraint check */
-static int regulator_check_voltage(struct regulator_dev *rdev,
-				   int *min_uV, int *max_uV)
+int regulator_check_voltage(struct regulator_dev *rdev,
+			    int *min_uV, int *max_uV)
 {
 	BUG_ON(*min_uV > *max_uV);
 
@@ -464,9 +460,9 @@ static int regulator_check_states(suspend_state_t state)
 /* Make sure we select a voltage that suits the needs of all
  * regulator consumers
  */
-static int regulator_check_consumers(struct regulator_dev *rdev,
-				     int *min_uV, int *max_uV,
-				     suspend_state_t state)
+int regulator_check_consumers(struct regulator_dev *rdev,
+			      int *min_uV, int *max_uV,
+			      suspend_state_t state)
 {
 	struct regulator *regulator;
 	struct regulator_voltage *voltage;
@@ -577,7 +573,7 @@ static ssize_t regulator_uV_show(struct device *dev,
 	ssize_t ret;
 
 	regulator_lock(rdev);
-	ret = sprintf(buf, "%d\n", _regulator_get_voltage(rdev));
+	ret = sprintf(buf, "%d\n", regulator_get_voltage_rdev(rdev));
 	regulator_unlock(rdev);
 
 	return ret;
@@ -948,7 +944,7 @@ static int drms_uA_update(struct regulator_dev *rdev)
 			rdev_err(rdev, "failed to set load %d\n", current_uA);
 	} else {
 		/* get output voltage */
-		output_uV = _regulator_get_voltage(rdev);
+		output_uV = regulator_get_voltage_rdev(rdev);
 		if (output_uV <= 0) {
 			rdev_err(rdev, "invalid output voltage found\n");
 			return -EINVAL;
@@ -1061,7 +1057,7 @@ static void print_constraints(struct regulator_dev *rdev)
 
 	if (!constraints->min_uV ||
 	    constraints->min_uV != constraints->max_uV) {
-		ret = _regulator_get_voltage(rdev);
+		ret = regulator_get_voltage_rdev(rdev);
 		if (ret > 0)
 			count += scnprintf(buf + count, len - count,
 					   "at %d mV ", ret / 1000);
@@ -1120,7 +1116,7 @@ static int machine_constraints_voltage(struct regulator_dev *rdev,
 	if (rdev->constraints->apply_uV &&
 	    rdev->constraints->min_uV && rdev->constraints->max_uV) {
 		int target_min, target_max;
-		int current_uV = _regulator_get_voltage(rdev);
+		int current_uV = regulator_get_voltage_rdev(rdev);
 
 		if (current_uV == -ENOTRECOVERABLE) {
 			/* This regulator can't be read and must be initialized */
@@ -1130,7 +1126,7 @@ static int machine_constraints_voltage(struct regulator_dev *rdev,
 			_regulator_do_set_voltage(rdev,
 						  rdev->constraints->min_uV,
 						  rdev->constraints->max_uV);
-			current_uV = _regulator_get_voltage(rdev);
+			current_uV = regulator_get_voltage_rdev(rdev);
 		}
 
 		if (current_uV < 0) {
@@ -3071,7 +3067,7 @@ static int _regulator_call_set_voltage(struct regulator_dev *rdev,
 	struct pre_voltage_change_data data;
 	int ret;
 
-	data.old_uV = _regulator_get_voltage(rdev);
+	data.old_uV = regulator_get_voltage_rdev(rdev);
 	data.min_uV = min_uV;
 	data.max_uV = max_uV;
 	ret = _notifier_call_chain(rdev, REGULATOR_EVENT_PRE_VOLTAGE_CHANGE,
@@ -3095,7 +3091,7 @@ static int _regulator_call_set_voltage_sel(struct regulator_dev *rdev,
 	struct pre_voltage_change_data data;
 	int ret;
 
-	data.old_uV = _regulator_get_voltage(rdev);
+	data.old_uV = regulator_get_voltage_rdev(rdev);
 	data.min_uV = uV;
 	data.max_uV = uV;
 	ret = _notifier_call_chain(rdev, REGULATOR_EVENT_PRE_VOLTAGE_CHANGE,
@@ -3148,7 +3144,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	unsigned int selector;
 	int old_selector = -1;
 	const struct regulator_ops *ops = rdev->desc->ops;
-	int old_uV = _regulator_get_voltage(rdev);
+	int old_uV = regulator_get_voltage_rdev(rdev);
 
 	trace_regulator_set_voltage(rdev_get_name(rdev), min_uV, max_uV);
 
@@ -3175,7 +3171,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 				best_val = ops->list_voltage(rdev,
 							     selector);
 			else
-				best_val = _regulator_get_voltage(rdev);
+				best_val = regulator_get_voltage_rdev(rdev);
 		}
 
 	} else if (ops->set_voltage_sel) {
@@ -3294,7 +3290,7 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 	 * changing the voltage.
 	 */
 	if (!regulator_ops_is_valid(rdev, REGULATOR_CHANGE_VOLTAGE)) {
-		current_uV = _regulator_get_voltage(rdev);
+		current_uV = regulator_get_voltage_rdev(rdev);
 		if (min_uV <= current_uV && current_uV <= max_uV) {
 			voltage->min_uV = min_uV;
 			voltage->max_uV = max_uV;
@@ -3331,8 +3327,8 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 	return ret;
 }
 
-static int regulator_set_voltage_rdev(struct regulator_dev *rdev, int min_uV,
-				      int max_uV, suspend_state_t state)
+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;
@@ -3360,7 +3356,7 @@ static int regulator_set_voltage_rdev(struct regulator_dev *rdev, int min_uV,
 
 		best_supply_uV += rdev->desc->min_dropout_uV;
 
-		current_supply_uV = _regulator_get_voltage(rdev->supply->rdev);
+		current_supply_uV = regulator_get_voltage_rdev(rdev->supply->rdev);
 		if (current_supply_uV < 0) {
 			ret = current_supply_uV;
 			goto out;
@@ -3411,7 +3407,7 @@ static int regulator_limit_voltage_step(struct regulator_dev *rdev,
 		return 1;
 
 	if (*current_uV < 0) {
-		*current_uV = _regulator_get_voltage(rdev);
+		*current_uV = regulator_get_voltage_rdev(rdev);
 
 		if (*current_uV < 0)
 			return *current_uV;
@@ -3515,7 +3511,7 @@ static int regulator_get_optimal_voltage(struct regulator_dev *rdev,
 		if (!_regulator_is_enabled(c_rdevs[i]))
 			continue;
 
-		tmp_act = _regulator_get_voltage(c_rdevs[i]);
+		tmp_act = regulator_get_voltage_rdev(c_rdevs[i]);
 		if (tmp_act < 0)
 			return tmp_act;
 
@@ -3557,7 +3553,7 @@ static int regulator_get_optimal_voltage(struct regulator_dev *rdev,
 	if (n_coupled > 1 && *current_uV == -1) {
 
 		if (_regulator_is_enabled(rdev)) {
-			ret = _regulator_get_voltage(rdev);
+			ret = regulator_get_voltage_rdev(rdev);
 			if (ret < 0)
 				return ret;
 
@@ -3929,7 +3925,7 @@ int regulator_sync_voltage(struct regulator *regulator)
 }
 EXPORT_SYMBOL_GPL(regulator_sync_voltage);
 
-static int _regulator_get_voltage(struct regulator_dev *rdev)
+int regulator_get_voltage_rdev(struct regulator_dev *rdev)
 {
 	int sel, ret;
 	bool bypassed;
@@ -3946,7 +3942,7 @@ static int _regulator_get_voltage(struct regulator_dev *rdev)
 				return -EPROBE_DEFER;
 			}
 
-			return _regulator_get_voltage(rdev->supply->rdev);
+			return regulator_get_voltage_rdev(rdev->supply->rdev);
 		}
 	}
 
@@ -3962,7 +3958,7 @@ static int _regulator_get_voltage(struct regulator_dev *rdev)
 	} else if (rdev->desc->fixed_uV && (rdev->desc->n_voltages == 1)) {
 		ret = rdev->desc->fixed_uV;
 	} else if (rdev->supply) {
-		ret = _regulator_get_voltage(rdev->supply->rdev);
+		ret = regulator_get_voltage_rdev(rdev->supply->rdev);
 	} else {
 		return -EINVAL;
 	}
@@ -3987,7 +3983,7 @@ int regulator_get_voltage(struct regulator *regulator)
 	int ret;
 
 	regulator_lock_dependent(regulator->rdev, &ww_ctx);
-	ret = _regulator_get_voltage(regulator->rdev);
+	ret = regulator_get_voltage_rdev(regulator->rdev);
 	regulator_unlock_dependent(regulator->rdev, &ww_ctx);
 
 	return ret;
@@ -5296,7 +5292,7 @@ static void regulator_summary_show_subtree(struct seq_file *s,
 		   rdev->use_count, rdev->open_count, rdev->bypass_count,
 		   regulator_opmode_to_str(opmode));
 
-	seq_printf(s, "%5dmV ", _regulator_get_voltage(rdev) / 1000);
+	seq_printf(s, "%5dmV ", regulator_get_voltage_rdev(rdev) / 1000);
 	seq_printf(s, "%5dmA ",
 		   _regulator_get_current_limit_unlocked(rdev) / 1000);
 
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 377da2357118..25c5a1862af2 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -20,6 +20,7 @@
 #include <linux/device.h>
 #include <linux/notifier.h>
 #include <linux/regulator/consumer.h>
+#include <linux/suspend.h>
 #include <linux/ww_mutex.h>
 
 struct gpio_desc;
@@ -552,4 +553,15 @@ void regulator_unlock(struct regulator_dev *rdev);
  */
 int regulator_desc_list_voltage_linear_range(const struct regulator_desc *desc,
 					     unsigned int selector);
+
+const char *rdev_get_name(struct regulator_dev *rdev);
+int regulator_check_consumers(struct regulator_dev *rdev,
+			      int *min_uV, int *max_uV,
+			      suspend_state_t state);
+int regulator_check_voltage(struct regulator_dev *rdev,
+			    int *min_uV, int *max_uV);
+int regulator_get_voltage_rdev(struct regulator_dev *rdev);
+int regulator_set_voltage_rdev(struct regulator_dev *rdev,
+			       int min_uV, int max_uV,
+			       suspend_state_t state);
 #endif
-- 
2.21.0

[RFC PATCH v1 4/6] regulator: core Bump MAX_COUPLED to 3

[Dmitry Osipenko]

NVIDIA Tegra20 SoC's couple 3 regulators, bump the MAX_COUPLED accordingly
to support that case of coupling.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 include/linux/regulator/driver.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 25c5a1862af2..25826f2b0ceb 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -15,7 +15,7 @@
 #ifndef __LINUX_REGULATOR_DRIVER_H_
 #define __LINUX_REGULATOR_DRIVER_H_
 
-#define MAX_COUPLED		2
+#define MAX_COUPLED		3
 
 #include <linux/device.h>
 #include <linux/notifier.h>
-- 
2.21.0

[RFC PATCH v1 5/6] soc/tegra: regulators: Add regulators coupler for Tegra20

[Dmitry Osipenko]

Add regulators coupler for Tegra20 SoC's that performs voltage balancing
of a coupled regulators and thus provides voltage scaling functionality.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/soc/tegra/Kconfig              |   6 +
 drivers/soc/tegra/Makefile             |   1 +
 drivers/soc/tegra/regulators-tegra20.c | 304 +++++++++++++++++++++++++
 3 files changed, 311 insertions(+)
 create mode 100644 drivers/soc/tegra/regulators-tegra20.c

diff --git a/drivers/soc/tegra/Kconfig b/drivers/soc/tegra/Kconfig
index a0b03443d8c1..545c0da2e069 100644
--- a/drivers/soc/tegra/Kconfig
+++ b/drivers/soc/tegra/Kconfig
@@ -133,3 +133,9 @@ config SOC_TEGRA_POWERGATE_BPMP
 	def_bool y
 	depends on PM_GENERIC_DOMAINS
 	depends on TEGRA_BPMP
+
+config SOC_TEGRA20_VOLTAGE_COUPLER
+	bool "Voltage scaling support for Tegra20 SoC's"
+	def_bool y
+	depends on ARCH_TEGRA_2x_SOC
+	depends on REGULATOR
diff --git a/drivers/soc/tegra/Makefile b/drivers/soc/tegra/Makefile
index 902759fe5f4d..9f0bdd53bef8 100644
--- a/drivers/soc/tegra/Makefile
+++ b/drivers/soc/tegra/Makefile
@@ -5,3 +5,4 @@ obj-y += common.o
 obj-$(CONFIG_SOC_TEGRA_FLOWCTRL) += flowctrl.o
 obj-$(CONFIG_SOC_TEGRA_PMC) += pmc.o
 obj-$(CONFIG_SOC_TEGRA_POWERGATE_BPMP) += powergate-bpmp.o
+obj-$(CONFIG_SOC_TEGRA20_VOLTAGE_COUPLER) += regulators-tegra20.o
diff --git a/drivers/soc/tegra/regulators-tegra20.c b/drivers/soc/tegra/regulators-tegra20.c
new file mode 100644
index 000000000000..3f005b804af3
--- /dev/null
+++ b/drivers/soc/tegra/regulators-tegra20.c
@@ -0,0 +1,304 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Voltage regulators coupling resolver for NVIDIA Tegra20
+ *
+ * Author: Dmitry Osipenko <digetx@gmail.com>
+ */
+
+#define pr_fmt(fmt)	"tegra voltage-coupler: " fmt
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+
+struct tegra_regulators_coupler {
+	struct regulators_coupler coupler;
+	int core_min_uV;
+};
+
+static const char * const cpu_names[] = {
+	"vdd_sys_sm0,vdd_core",
+	"+1.0vs_sm1,vdd_cpu",
+	"vdd_sm1,vdd_cpu",
+	"VDD_CPU_1.0V",
+	"vdd_cpu",
+};
+
+static const char * const core_names[] = {
+	"vdd_sys_sm1,vdd_cpu",
+	"+1.2vs_sm0,vdd_core",
+	"vdd_sm0,vdd_core",
+	"VDD_CORE_1.2V",
+	"vdd_core",
+};
+
+static const char * const rtc_names[] = {
+	"+1.2vs_ldo2,vdd_rtc",
+	"vdd_ldo2,vdd_rtc",
+	"VDD_RTC_1.2V",
+	"vdd_rtc",
+};
+
+static inline struct tegra_regulators_coupler *
+to_tegra_coupler(struct regulators_coupler *coupler)
+{
+	return container_of(coupler, struct tegra_regulators_coupler, coupler);
+}
+
+static int tegra20_core_limit(struct tegra_regulators_coupler *tegra,
+			      struct regulator_dev *core_rdev,
+			      struct regulator_dev *rtc_rdev)
+{
+	int core_min_uV;
+
+	if (tegra->core_min_uV > 0)
+		return tegra->core_min_uV;
+
+	core_min_uV = regulator_get_voltage_rdev(core_rdev);
+	if (core_min_uV > 0) {
+		pr_info("core minimum voltage limited to %duV\n", core_min_uV);
+		tegra->core_min_uV = core_min_uV;
+	}
+
+	return core_min_uV;
+}
+
+static int tegra20_core_rtc_update(struct tegra_regulators_coupler *tegra,
+				   struct regulator_dev *core_rdev,
+				   struct regulator_dev *rtc_rdev,
+				   int cpu_uV, int cpu_min_uV)
+{
+	int core_min_uV, core_max_uV = INT_MAX;
+	int rtc_min_uV, rtc_max_uV = INT_MAX;
+	int core_target_uV;
+	int rtc_target_uV;
+	int core_uV;
+	int rtc_uV;
+	int err;
+
+	/*
+	 * The core voltage scaling is currently not hooked up in drivers,
+	 * hence we will limit the minimum core voltage to the initial value.
+	 * This should be good enough for the time being.
+	 */
+	core_min_uV = tegra20_core_limit(tegra, core_rdev, rtc_rdev);
+	if (core_min_uV < 0)
+		return core_min_uV;
+
+	err = regulator_check_voltage(core_rdev, &core_min_uV, &core_max_uV);
+	if (err)
+		return err;
+
+	err = regulator_check_consumers(core_rdev, &core_min_uV, &core_max_uV,
+					PM_SUSPEND_ON);
+	if (err)
+		return err;
+
+	core_min_uV = max(cpu_min_uV + 125000, core_min_uV);
+	if (core_min_uV > core_max_uV)
+		return -EINVAL;
+
+	core_uV = regulator_get_voltage_rdev(core_rdev);
+	if (core_uV < 0)
+		return core_uV;
+
+	if (cpu_uV + 120000 > core_uV)
+		pr_err("core-cpu voltage constraint violated: %d %d\n",
+		       core_uV, cpu_uV + 120000);
+
+	rtc_uV = regulator_get_voltage_rdev(rtc_rdev);
+	if (rtc_uV < 0)
+		return rtc_uV;
+
+	if (cpu_uV + 120000 > rtc_uV)
+		pr_err("rtc-cpu voltage constraint violated: %d %d\n",
+		       rtc_uV, cpu_uV + 120000);
+
+	if (abs(core_uV - rtc_uV) > 170000)
+		pr_err("core-rtc voltage constraint violated: %d %d\n",
+		       core_uV, rtc_uV);
+
+	rtc_min_uV = max(cpu_min_uV + 125000, core_min_uV - 150000);
+
+	err = regulator_check_voltage(rtc_rdev, &rtc_min_uV, &rtc_max_uV);
+	if (err)
+		return err;
+
+	while (core_uV != core_min_uV || rtc_uV != rtc_min_uV) {
+		if (core_uV < core_min_uV) {
+			core_target_uV = min(core_uV + 150000, core_min_uV);
+			core_target_uV = min(rtc_uV + 150000, core_target_uV);
+		} else {
+			core_target_uV = max(core_uV - 150000, core_min_uV);
+			core_target_uV = max(rtc_uV - 150000, core_target_uV);
+		}
+
+		err = regulator_set_voltage_rdev(core_rdev,
+						 core_target_uV,
+						 core_max_uV,
+						 PM_SUSPEND_ON);
+		if (err)
+			return err;
+
+		core_uV = core_target_uV;
+
+		if (rtc_uV < rtc_min_uV) {
+			rtc_target_uV = min(rtc_uV + 150000, rtc_min_uV);
+			rtc_target_uV = min(core_uV + 150000, rtc_target_uV);
+		} else {
+			rtc_target_uV = max(rtc_uV - 150000, rtc_min_uV);
+			rtc_target_uV = max(core_uV - 150000, rtc_target_uV);
+		}
+
+		err = regulator_set_voltage_rdev(rtc_rdev,
+						 rtc_target_uV,
+						 rtc_max_uV,
+						 PM_SUSPEND_ON);
+		if (err)
+			return err;
+
+		rtc_uV = rtc_target_uV;
+	}
+
+	return 0;
+}
+
+static int tegra20_core_voltage_update(struct tegra_regulators_coupler *tegra,
+				       struct regulator_dev *cpu_rdev,
+				       struct regulator_dev *core_rdev,
+				       struct regulator_dev *rtc_rdev)
+{
+	int cpu_uV;
+
+	cpu_uV = regulator_get_voltage_rdev(cpu_rdev);
+	if (cpu_uV < 0)
+		return cpu_uV;
+
+	return tegra20_core_rtc_update(tegra, core_rdev, rtc_rdev,
+				       cpu_uV, cpu_uV);
+}
+
+static int tegra20_cpu_voltage_update(struct tegra_regulators_coupler *tegra,
+				      struct regulator_dev *cpu_rdev,
+				      struct regulator_dev *core_rdev,
+				      struct regulator_dev *rtc_rdev)
+{
+	int cpu_min_uV = 0;
+	int cpu_max_uV = INT_MAX;
+	int cpu_uV;
+	int err;
+
+	err = regulator_check_voltage(cpu_rdev, &cpu_min_uV, &cpu_max_uV);
+	if (err)
+		return err;
+
+	err = regulator_check_consumers(cpu_rdev, &cpu_min_uV, &cpu_max_uV,
+					PM_SUSPEND_ON);
+	if (err)
+		return err;
+
+	cpu_uV = regulator_get_voltage_rdev(cpu_rdev);
+	if (cpu_uV < 0)
+		return cpu_uV;
+
+	if (cpu_min_uV > cpu_uV) {
+		err = tegra20_core_rtc_update(tegra, core_rdev, rtc_rdev,
+					      cpu_uV, cpu_min_uV);
+		if (err)
+			return err;
+
+		err = regulator_set_voltage_rdev(cpu_rdev, cpu_min_uV,
+						 cpu_max_uV, PM_SUSPEND_ON);
+		if (err)
+			return err;
+	} else if (cpu_min_uV < cpu_uV)  {
+		err = regulator_set_voltage_rdev(cpu_rdev, cpu_min_uV,
+						 cpu_max_uV, PM_SUSPEND_ON);
+		if (err)
+			return err;
+
+		err = tegra20_core_rtc_update(tegra, core_rdev, rtc_rdev,
+					      cpu_uV, cpu_min_uV);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static struct regulator_dev *lookup_rdev(struct regulator_dev *rdev,
+					 const char * const *names,
+					 unsigned int num_names)
+{
+	struct coupling_desc *c_desc = &rdev->coupling_desc;
+	unsigned int i, k;
+
+	for (i = 0; i < num_names; i++) {
+		if (!strcmp(names[i], rdev_get_name(rdev)))
+			return rdev;
+	}
+
+	for (k = 0; k < c_desc->n_coupled; k++) {
+		rdev = c_desc->coupled_rdevs[k];
+
+		for (i = 0; i < num_names; i++) {
+			if (!strcmp(names[i], rdev_get_name(rdev)))
+				return rdev;
+		}
+	}
+
+	pr_err_once("%s: failed for %s\n", __func__, rdev_get_name(rdev));
+
+	for (i = 0; i < num_names; i++)
+		pr_err_once("%s: entry%u: %s\n", __func__, i, names[i]);
+
+	return NULL;
+}
+
+static int tegra20_regulator_balance_voltage(struct regulators_coupler *coupler,
+					     struct regulator_dev *rdev,
+					     suspend_state_t state)
+{
+	struct tegra_regulators_coupler *tegra = to_tegra_coupler(coupler);
+	struct regulator_dev *core_rdev;
+	struct regulator_dev *cpu_rdev;
+	struct regulator_dev *rtc_rdev;
+
+	core_rdev = lookup_rdev(rdev, core_names, ARRAY_SIZE(core_names));
+	cpu_rdev  = lookup_rdev(rdev, cpu_names, ARRAY_SIZE(cpu_names));
+	rtc_rdev  = lookup_rdev(rdev, rtc_names, ARRAY_SIZE(rtc_names));
+
+	if (!core_rdev || !cpu_rdev || !rtc_rdev || state != PM_SUSPEND_ON) {
+		pr_err("regulators are not coupled properly\n");
+		return -EINVAL;
+	}
+
+	if (rdev == cpu_rdev)
+		return tegra20_cpu_voltage_update(tegra, cpu_rdev,
+						  core_rdev, rtc_rdev);
+
+	if (rdev == core_rdev)
+		return tegra20_core_voltage_update(tegra, cpu_rdev,
+						   core_rdev, rtc_rdev);
+
+	pr_err("driving %s voltage not permitted\n", rdev_get_name(rtc_rdev));
+
+	return -EPERM;
+}
+
+static struct tegra_regulators_coupler tegra20_coupler = {
+	.coupler = {
+		.balance_voltage = tegra20_regulator_balance_voltage,
+	},
+};
+
+static int __init tegra_regulators_coupler_init(void)
+{
+	if (!of_machine_is_compatible("nvidia,tegra20"))
+		return 0;
+
+	return regulators_coupler_register(&tegra20_coupler.coupler);
+}
+arch_initcall(tegra_regulators_coupler_init);
-- 
2.21.0

Re: [RFC PATCH v1 5/6] soc/tegra: regulators: Add regulators coupler for Tegra20

[Mark Brown]

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

On Sun, Apr 14, 2019 at 08:59:38PM +0300, Dmitry Osipenko wrote:
> Add regulators coupler for Tegra20 SoC's that performs voltage balancing
> of a coupled regulators and thus provides voltage scaling functionality.

Can you say what the rules that this is trying to follow are?

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

Re: [RFC PATCH v1 5/6] soc/tegra: regulators: Add regulators coupler for Tegra20

[Dmitry Osipenko]

08.05.2019 10:57, Mark Brown пишет:
> On Sun, Apr 14, 2019 at 08:59:38PM +0300, Dmitry Osipenko wrote:
>> Add regulators coupler for Tegra20 SoC's that performs voltage balancing
>> of a coupled regulators and thus provides voltage scaling functionality.
> 
> Can you say what the rules that this is trying to follow are?
> 

There are three regulators: CPU, CORE and RTC.

Constraints:

1) CORE and RTC have max-spread voltage of 170mV.
2) CORE and RTC voltages must be higher than the CPU voltage by at least
120mV.

Re: [RFC PATCH v1 5/6] soc/tegra: regulators: Add regulators coupler for Tegra20

[Mark Brown]

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

On Wed, May 08, 2019 at 04:10:58PM +0300, Dmitry Osipenko wrote:

> 1) CORE and RTC have max-spread voltage of 170mV.
> 2) CORE and RTC voltages must be higher than the CPU voltage by at least
> 120mV.

This seems like it should be easy enough to describe - we just need
minimum and maximum spreads between pairs of rails.

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

Re: [RFC PATCH v1 5/6] soc/tegra: regulators: Add regulators coupler for Tegra20

[Dmitry Osipenko]

12.05.2019 12:06, Mark Brown пишет:
> On Wed, May 08, 2019 at 04:10:58PM +0300, Dmitry Osipenko wrote:
> 
>> 1) CORE and RTC have max-spread voltage of 170mV.
>> 2) CORE and RTC voltages must be higher than the CPU voltage by at least
>> 120mV.
> 
> This seems like it should be easy enough to describe - we just need
> minimum and maximum spreads between pairs of rails.
> 

Yes, but the proper CORE/RTC minimum voltages shall be maintained until
all drivers will get support for the voltage management, which likely to
take a lot of time to get upstreamed. So I'd want to get at least some
basics working for the start, later on it should be possible to consider
generalization of the regulators coupling. Mark, are you okay with
having the custom regulators coupler as an interim solution?

Re: [RFC PATCH v1 5/6] soc/tegra: regulators: Add regulators coupler for Tegra20

[Mark Brown]

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

On Sun, May 12, 2019 at 08:42:39PM +0300, Dmitry Osipenko wrote:
> 12.05.2019 12:06, Mark Brown пишет:

> > This seems like it should be easy enough to describe - we just need
> > minimum and maximum spreads between pairs of rails.

> Yes, but the proper CORE/RTC minimum voltages shall be maintained until
> all drivers will get support for the voltage management, which likely to
> take a lot of time to get upstreamed. So I'd want to get at least some
> basics working for the start, later on it should be possible to consider
> generalization of the regulators coupling. Mark, are you okay with
> having the custom regulators coupler as an interim solution?

Let me think about it.  Interim solutions have this habit of hanging
around and the bit with needing to get all the drivers loaded is very
much an open and substantial question...  :/  Definitely not something
I'd close the door on at this point though.

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

Re: [RFC PATCH v1 5/6] soc/tegra: regulators: Add regulators coupler for Tegra20

[Dmitry Osipenko]

13.05.2019 20:38, Mark Brown пишет:
> On Sun, May 12, 2019 at 08:42:39PM +0300, Dmitry Osipenko wrote:
>> 12.05.2019 12:06, Mark Brown пишет:
> 
>>> This seems like it should be easy enough to describe - we just need
>>> minimum and maximum spreads between pairs of rails.
> 
>> Yes, but the proper CORE/RTC minimum voltages shall be maintained until
>> all drivers will get support for the voltage management, which likely to
>> take a lot of time to get upstreamed. So I'd want to get at least some
>> basics working for the start, later on it should be possible to consider
>> generalization of the regulators coupling. Mark, are you okay with
>> having the custom regulators coupler as an interim solution?
> 
> Let me think about it.  Interim solutions have this habit of hanging
> around and the bit with needing to get all the drivers loaded is very
> much an open and substantial question...  :/  Definitely not something
> I'd close the door on at this point though.
> 

This one has a good chance to stick around for a substantial time.

Realistically I see two variants right now:

  1) get at least some basics to work (regulators coupling, CPUFreq
voltage managing) and then continue step-by-step

  2) give up on it all in upstream because likely that an immediate
complete solution will take just too much time and effort for a one
person to cope (I have other things to do as well)

Mark, I'm glad that you're not strongly opposed. Will prepare  v2.

If anyone else has something to say, please don't shy.

[RFC PATCH v1 6/6] soc/tegra: regulators: Add regulators coupler for Tegra30

[Dmitry Osipenko]

Add regulators coupler for Tegra30 SoC's that performs voltage balancing
of a coupled regulators and thus provides voltage scaling functionality.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/soc/tegra/Kconfig              |   6 +
 drivers/soc/tegra/Makefile             |   1 +
 drivers/soc/tegra/regulators-tegra30.c | 256 +++++++++++++++++++++++++
 3 files changed, 263 insertions(+)
 create mode 100644 drivers/soc/tegra/regulators-tegra30.c

diff --git a/drivers/soc/tegra/Kconfig b/drivers/soc/tegra/Kconfig
index 545c0da2e069..a5235af27644 100644
--- a/drivers/soc/tegra/Kconfig
+++ b/drivers/soc/tegra/Kconfig
@@ -139,3 +139,9 @@ config SOC_TEGRA20_VOLTAGE_COUPLER
 	def_bool y
 	depends on ARCH_TEGRA_2x_SOC
 	depends on REGULATOR
+
+config SOC_TEGRA30_VOLTAGE_COUPLER
+	bool "Voltage scaling support for Tegra30 SoC's"
+	def_bool y
+	depends on ARCH_TEGRA_3x_SOC
+	depends on REGULATOR
diff --git a/drivers/soc/tegra/Makefile b/drivers/soc/tegra/Makefile
index 9f0bdd53bef8..9c809c1814bd 100644
--- a/drivers/soc/tegra/Makefile
+++ b/drivers/soc/tegra/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_SOC_TEGRA_FLOWCTRL) += flowctrl.o
 obj-$(CONFIG_SOC_TEGRA_PMC) += pmc.o
 obj-$(CONFIG_SOC_TEGRA_POWERGATE_BPMP) += powergate-bpmp.o
 obj-$(CONFIG_SOC_TEGRA20_VOLTAGE_COUPLER) += regulators-tegra20.o
+obj-$(CONFIG_SOC_TEGRA30_VOLTAGE_COUPLER) += regulators-tegra30.o
diff --git a/drivers/soc/tegra/regulators-tegra30.c b/drivers/soc/tegra/regulators-tegra30.c
new file mode 100644
index 000000000000..ecd6a984e045
--- /dev/null
+++ b/drivers/soc/tegra/regulators-tegra30.c
@@ -0,0 +1,256 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Voltage regulators coupling resolver for NVIDIA Tegra30
+ *
+ * Author: Dmitry Osipenko <digetx@gmail.com>
+ */
+
+#define pr_fmt(fmt)	"tegra voltage-coupler: " fmt
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+
+#include <soc/tegra/fuse.h>
+
+struct tegra_regulators_coupler {
+	struct regulators_coupler coupler;
+	int core_min_uV;
+};
+
+static const char * const cpu_names[] = {
+	"vdd_cpu,vdd_sys",
+	"+V1.0_VDD_CPU",
+	"vdd_cpu",
+};
+
+static const char * const core_names[] = {
+	"tps62361-vout",
+	"tps62362-vout",
+	"vdd_core",
+};
+
+static inline struct tegra_regulators_coupler *
+to_tegra_coupler(struct regulators_coupler *coupler)
+{
+	return container_of(coupler, struct tegra_regulators_coupler, coupler);
+}
+
+static int tegra30_core_limit(struct tegra_regulators_coupler *tegra,
+			      struct regulator_dev *core_rdev)
+{
+	if (tegra->core_min_uV > 0)
+		return tegra->core_min_uV;
+
+	tegra->core_min_uV = regulator_get_voltage_rdev(core_rdev);
+	if (tegra->core_min_uV > 0)
+		pr_info("core minimum voltage limited to %duV\n",
+			tegra->core_min_uV);
+
+	return tegra->core_min_uV;
+}
+
+static int tegra30_core_cpu_limit(int cpu_uV)
+{
+	if (cpu_uV < 800000)
+		return 950000;
+
+	if (cpu_uV < 900000)
+		return 1000000;
+
+	if (cpu_uV < 1000000)
+		return 1100000;
+
+	if (cpu_uV < 1100000)
+		return 1200000;
+
+	if (cpu_uV < 1250000) {
+		switch (tegra_sku_info.cpu_speedo_id) {
+		case 0 ... 1:
+		case 4:
+		case 7:
+		case 8:
+			return 1200000;
+
+		default:
+			return 1300000;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int tegra30_voltage_update(struct tegra_regulators_coupler *tegra,
+				  struct regulator_dev *cpu_rdev,
+				  struct regulator_dev *core_rdev)
+{
+	int cpu_min_uV, cpu_max_uV = INT_MAX;
+	int core_min_uV, core_max_uV = INT_MAX;
+	int core_min_limited_uV;
+	int core_target_uV;
+	int cpu_target_uV;
+	int core_uV;
+	int cpu_uV;
+	int err;
+
+	/*
+	 * The CORE voltage scaling is currently not hooked up in drivers,
+	 * hence we will limit the minimum CORE voltage to the initial value.
+	 * This should be good enough for the time being.
+	 */
+	core_min_uV = tegra30_core_limit(tegra, core_rdev);
+	if (core_min_uV < 0)
+		return core_min_uV;
+
+	err = regulator_check_consumers(core_rdev, &core_min_uV, &core_max_uV,
+					PM_SUSPEND_ON);
+	if (err)
+		return err;
+
+	cpu_min_uV = core_min_uV - 300000;
+
+	err = regulator_check_consumers(cpu_rdev, &cpu_min_uV, &cpu_max_uV,
+					PM_SUSPEND_ON);
+	if (err)
+		return err;
+
+	err = regulator_check_voltage(cpu_rdev, &cpu_min_uV, &cpu_max_uV);
+	if (err)
+		return err;
+
+	cpu_uV = regulator_get_voltage_rdev(cpu_rdev);
+	if (cpu_uV < 0)
+		return cpu_uV;
+
+	core_uV = regulator_get_voltage_rdev(core_rdev);
+	if (core_uV < 0)
+		return core_uV;
+
+	/*
+	 * Bootloader shall set up voltages correctly, but if it
+	 * happens that there is a violation, then try to fix it
+	 * at first.
+	 */
+	core_min_limited_uV = tegra30_core_cpu_limit(cpu_uV);
+	if (core_min_limited_uV < 0)
+		return core_min_limited_uV;
+
+	core_min_uV = max(core_min_uV, tegra30_core_cpu_limit(cpu_min_uV));
+
+	err = regulator_check_voltage(core_rdev, &core_min_uV, &core_max_uV);
+	if (err)
+		return err;
+
+	if (core_min_limited_uV > core_uV) {
+		pr_err("core voltage constraint violated: %d %d %d\n",
+		       core_uV, core_min_limited_uV, cpu_uV);
+		goto update_core;
+	}
+
+	while (cpu_uV != cpu_min_uV || core_uV != core_min_uV) {
+		if (cpu_uV < cpu_min_uV) {
+			cpu_target_uV = min(cpu_uV + 100000, cpu_min_uV);
+		} else {
+			cpu_target_uV = max(cpu_uV - 100000, cpu_min_uV);
+			cpu_target_uV = max(core_uV - 300000, cpu_target_uV);
+		}
+
+		err = regulator_set_voltage_rdev(cpu_rdev,
+						 cpu_target_uV,
+						 cpu_max_uV,
+						 PM_SUSPEND_ON);
+		if (err)
+			return err;
+
+		cpu_uV = cpu_target_uV;
+update_core:
+		core_min_limited_uV = tegra30_core_cpu_limit(cpu_uV);
+		if (core_min_limited_uV < 0)
+			return core_min_limited_uV;
+
+		core_target_uV = max(core_min_limited_uV, core_min_uV);
+
+		if (core_uV < core_target_uV) {
+			core_target_uV = min(core_target_uV, core_uV + 100000);
+			core_target_uV = min(core_target_uV, cpu_uV + 300000);
+		} else {
+			core_target_uV = max(core_target_uV, core_uV - 100000);
+		}
+
+		err = regulator_set_voltage_rdev(core_rdev,
+						 core_target_uV,
+						 core_max_uV,
+						 PM_SUSPEND_ON);
+		if (err)
+			return err;
+
+		core_uV = core_target_uV;
+	}
+
+	return 0;
+}
+
+static struct regulator_dev *lookup_rdev(struct regulator_dev *rdev,
+					 const char * const *names,
+					 unsigned int num_names)
+{
+	struct coupling_desc *c_desc = &rdev->coupling_desc;
+	unsigned int i, k;
+
+	for (i = 0; i < num_names; i++) {
+		if (!strcmp(names[i], rdev_get_name(rdev)))
+			return rdev;
+	}
+
+	for (k = 0; k < c_desc->n_coupled; k++) {
+		rdev = c_desc->coupled_rdevs[k];
+
+		for (i = 0; i < num_names; i++) {
+			if (!strcmp(names[i], rdev_get_name(rdev)))
+				return rdev;
+		}
+	}
+
+	pr_err_once("%s: failed for %s\n", __func__, rdev_get_name(rdev));
+
+	for (i = 0; i < num_names; i++)
+		pr_err_once("%s: entry%u: %s\n", __func__, i, names[i]);
+
+	return NULL;
+}
+
+static int tegra30_regulator_balance_voltage(struct regulators_coupler *coupler,
+					     struct regulator_dev *rdev,
+					     suspend_state_t state)
+{
+	struct tegra_regulators_coupler *tegra = to_tegra_coupler(coupler);
+	struct regulator_dev *core_rdev;
+	struct regulator_dev *cpu_rdev;
+
+	core_rdev = lookup_rdev(rdev, core_names, ARRAY_SIZE(core_names));
+	cpu_rdev  = lookup_rdev(rdev, cpu_names, ARRAY_SIZE(cpu_names));
+
+	if (!core_rdev || !cpu_rdev || state != PM_SUSPEND_ON) {
+		pr_err("regulators are not coupled properly\n");
+		return -EINVAL;
+	}
+
+	return tegra30_voltage_update(tegra, cpu_rdev, core_rdev);
+}
+
+static struct tegra_regulators_coupler tegra30_coupler = {
+	.coupler = {
+		.balance_voltage = tegra30_regulator_balance_voltage,
+	},
+};
+
+static int __init tegra_regulators_coupler_init(void)
+{
+	if (!of_machine_is_compatible("nvidia,tegra30"))
+		return 0;
+
+	return regulators_coupler_register(&tegra30_coupler.coupler);
+}
+arch_initcall(tegra_regulators_coupler_init);
-- 
2.21.0

Re: [RFC PATCH v1 6/6] soc/tegra: regulators: Add regulators coupler for Tegra30

[Mark Brown]

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

On Sun, Apr 14, 2019 at 08:59:39PM +0300, Dmitry Osipenko wrote:
> Add regulators coupler for Tegra30 SoC's that performs voltage balancing
> of a coupled regulators and thus provides voltage scaling functionality.

Same here, what are the requirements this is implementing?

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

Re: [RFC PATCH v1 6/6] soc/tegra: regulators: Add regulators coupler for Tegra30

[Dmitry Osipenko]

08.05.2019 10:58, Mark Brown пишет:
> On Sun, Apr 14, 2019 at 08:59:39PM +0300, Dmitry Osipenko wrote:
>> Add regulators coupler for Tegra30 SoC's that performs voltage balancing
>> of a coupled regulators and thus provides voltage scaling functionality.
> 
> Same here, what are the requirements this is implementing?
> 

There are two coupled regulators: CPU and CORE.

Constraints:

1) The max-spread voltage is 300mV.

2) CORE voltage must be higher than the CPU by at least N mV, where N
varies depending on the CPU voltage.

3) There is a constraint on the maximum CORE voltage depending on
hardware model/revision (cpu_speedo_id) where a higher voltages
apparently may cause physical damage, so it's better to hardcode the
limitation in the code rather than to rely on a board's device-tree
description. This constraint is quite vaguely defined in the downstream
kernel, I'm not really sure if it's solely about the hardware safety.

Re: [RFC PATCH v1 6/6] soc/tegra: regulators: Add regulators coupler for Tegra30

[Mark Brown]

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

On Wed, May 08, 2019 at 04:27:42PM +0300, Dmitry Osipenko wrote:

> Constraints:

> 1) The max-spread voltage is 300mV.

> 2) CORE voltage must be higher than the CPU by at least N mV, where N
> varies depending on the CPU voltage.

Those seem like they should be doable in generic code, though the fact
that the constraint is variable makes it annoying to specify - otherwise
it'd just be a minimum and maximum spread.  I'm not really coming up
with any great ideas right now, it's getting into OPP type territory but
it sounds like there's more flexibility for ramping the core voltage so
you'd end up with silly numbers of OPPs.

> 3) There is a constraint on the maximum CORE voltage depending on
> hardware model/revision (cpu_speedo_id) where a higher voltages
> apparently may cause physical damage, so it's better to hardcode the
> limitation in the code rather than to rely on a board's device-tree
> description. This constraint is quite vaguely defined in the downstream
> kernel, I'm not really sure if it's solely about the hardware safety.

I'd expect this to be enforced by the cpufreq driver just not selecting
higher voltages on affected parts.

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

Re: [RFC PATCH v1 6/6] soc/tegra: regulators: Add regulators coupler for Tegra30

[Dmitry Osipenko]

12.05.2019 12:04, Mark Brown пишет:
> On Wed, May 08, 2019 at 04:27:42PM +0300, Dmitry Osipenko wrote:
> 
>> Constraints:
> 
>> 1) The max-spread voltage is 300mV.
> 
>> 2) CORE voltage must be higher than the CPU by at least N mV, where N
>> varies depending on the CPU voltage.
> 
> Those seem like they should be doable in generic code, though the fact
> that the constraint is variable makes it annoying to specify - otherwise
> it'd just be a minimum and maximum spread.  I'm not really coming up
> with any great ideas right now, it's getting into OPP type territory but
> it sounds like there's more flexibility for ramping the core voltage so
> you'd end up with silly numbers of OPPs.

The OPP shouldn't have to do anything in regards to the regulators
coupling. The whole idea of the regulators coupling is to make device
drivers to not churn with the coupling. The coupling in this case is
specific to SoC and not to a particular board.

I think the current approach with the customized regulators coupler is
the best solution for the time being. We may consider something more
generic if there will be other users with a similar coupling
requirements, otherwise it's quite difficult to judge what is "generic".
Do you agree?

>> 3) There is a constraint on the maximum CORE voltage depending on
>> hardware model/revision (cpu_speedo_id) where a higher voltages
>> apparently may cause physical damage, so it's better to hardcode the
>> limitation in the code rather than to rely on a board's device-tree
>> description. This constraint is quite vaguely defined in the downstream
>> kernel, I'm not really sure if it's solely about the hardware safety.
> 
> I'd expect this to be enforced by the cpufreq driver just not selecting
> higher voltages on affected parts.
> 

CPUFreq driver will only handle the CPU regulator and it won't know
anything about the CORE.

Anyway, please scratch the third constraint, I messed up it with the
other *minimum* CORE voltage constraint detail which makes the minimum
voltage to depend on the hardware version in addition to the CPU voltage.

Re: [RFC PATCH v1 6/6] soc/tegra: regulators: Add regulators coupler for Tegra30

[Mark Brown]

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

On Sun, May 12, 2019 at 09:29:54PM +0300, Dmitry Osipenko wrote:
> 12.05.2019 12:04, Mark Brown пишет:
> > On Wed, May 08, 2019 at 04:27:42PM +0300, Dmitry Osipenko wrote:

> > Those seem like they should be doable in generic code, though the fact
> > that the constraint is variable makes it annoying to specify - otherwise
> > it'd just be a minimum and maximum spread.  I'm not really coming up
> > with any great ideas right now, it's getting into OPP type territory but
> > it sounds like there's more flexibility for ramping the core voltage so
> > you'd end up with silly numbers of OPPs.

> The OPP shouldn't have to do anything in regards to the regulators
> coupling. The whole idea of the regulators coupling is to make device
> drivers to not churn with the coupling. The coupling in this case is
> specific to SoC and not to a particular board.

The thing with OPPs is that they specify a whole table of values that
work together including regulator settings, the result being that you
have many fewer options but don't need to think about constraints.

> I think the current approach with the customized regulators coupler is
> the best solution for the time being. We may consider something more
> generic if there will be other users with a similar coupling
> requirements, otherwise it's quite difficult to judge what is "generic".
> Do you agree?

Some of the constraints (like having drivers loaded) are kind of fun...

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

Re: [RFC PATCH v1 6/6] soc/tegra: regulators: Add regulators coupler for Tegra30

[Dmitry Osipenko]

13.05.2019 20:40, Mark Brown пишет:
> On Sun, May 12, 2019 at 09:29:54PM +0300, Dmitry Osipenko wrote:
>> 12.05.2019 12:04, Mark Brown пишет:
>>> On Wed, May 08, 2019 at 04:27:42PM +0300, Dmitry Osipenko wrote:
> 
>>> Those seem like they should be doable in generic code, though the fact
>>> that the constraint is variable makes it annoying to specify - otherwise
>>> it'd just be a minimum and maximum spread.  I'm not really coming up
>>> with any great ideas right now, it's getting into OPP type territory but
>>> it sounds like there's more flexibility for ramping the core voltage so
>>> you'd end up with silly numbers of OPPs.
> 
>> The OPP shouldn't have to do anything in regards to the regulators
>> coupling. The whole idea of the regulators coupling is to make device
>> drivers to not churn with the coupling. The coupling in this case is
>> specific to SoC and not to a particular board.
> 
> The thing with OPPs is that they specify a whole table of values that
> work together including regulator settings, the result being that you
> have many fewer options but don't need to think about constraints.

I'm afraid this is just a way of abusing the OPP's. I actually already
had variant of the CPUFreq driver where it was managing all of the
coupled regulators and gave up on it because it's just not very
practical and adds a lot of unnecessary churning into the code. Note
that it's just the CPUFreq driver, there are quite a lot of other (CORE)
drivers as well and there are a lot of voltage combinations because OPP
entries are also specific to a range of hardware versions.

>> I think the current approach with the customized regulators coupler is
>> the best solution for the time being. We may consider something more
>> generic if there will be other users with a similar coupling
>> requirements, otherwise it's quite difficult to judge what is "generic".
>> Do you agree?
> 
> Some of the constraints (like having drivers loaded) are kind of fun...
> 

AFAIK, there is no good solution in upstream kernel for that problem
yet. Maybe it will be possible to reset hardware into a some more
predictable state early during kernel's boot for the start, will see.

Re: [RFC PATCH v1 6/6] soc/tegra: regulators: Add regulators coupler for Tegra30

[Mark Brown]

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

On Tue, May 14, 2019 at 09:30:05PM +0300, Dmitry Osipenko wrote:
> 13.05.2019 20:40, Mark Brown пишет:

> > The thing with OPPs is that they specify a whole table of values that
> > work together including regulator settings, the result being that you
> > have many fewer options but don't need to think about constraints.

> I'm afraid this is just a way of abusing the OPP's. I actually already

There's nothing wrong with handling regulators in an OPP, that's a
totally normal thing.

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

Re: [RFC PATCH v1 6/6] soc/tegra: regulators: Add regulators coupler for Tegra30

[Dmitry Osipenko]

15.05.2019 12:05, Mark Brown пишет:
> On Tue, May 14, 2019 at 09:30:05PM +0300, Dmitry Osipenko wrote:
>> 13.05.2019 20:40, Mark Brown пишет:
> 
>>> The thing with OPPs is that they specify a whole table of values that
>>> work together including regulator settings, the result being that you
>>> have many fewer options but don't need to think about constraints.
> 
>> I'm afraid this is just a way of abusing the OPP's. I actually already
> 
> There's nothing wrong with handling regulators in an OPP, that's a
> totally normal thing.
> 

Only if those regulators are directly related to the hardware unit,
which is not the case here. Regulators coupling is the right abstraction
that glues things together, there is absolutely no need in trying to
make workarounds using OPP.

Re: [RFC PATCH v1 6/6] soc/tegra: regulators: Add regulators coupler for Tegra30

[Mark Brown]

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

On Wed, May 15, 2019 at 02:44:33PM +0300, Dmitry Osipenko wrote:
> 15.05.2019 12:05, Mark Brown пишет:
> > On Tue, May 14, 2019 at 09:30:05PM +0300, Dmitry Osipenko wrote:

> >> I'm afraid this is just a way of abusing the OPP's. I actually already

> > There's nothing wrong with handling regulators in an OPP, that's a
> > totally normal thing.

> Only if those regulators are directly related to the hardware unit,
> which is not the case here. Regulators coupling is the right abstraction
> that glues things together, there is absolutely no need in trying to
> make workarounds using OPP.

The thing with OPPs is that they are often system level rather than
related to one specific parrt of the device - one of the reasons people
use them is that they eliminate the needs to think about dynamic
combinations of things and instead pick a suitable configuration off a
relatively short menu.  This makes both validation and runtime easier.

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

Re: [RFC PATCH v1 0/6] Introduce machine-specific regulators coupling API

[Dmitry Osipenko]

14.04.2019 20:59, Dmitry Osipenko пишет:
> Hello,
> 
> I was looking into how to properly implement regulators coupling for
> NVIDIA Tegra SoC's and ended up with this patchset that introduces
> machine-specific regulators coupling. Upstream kernel now has support
> for a simple variants of regulators coupling in a form of limiting
> maximum voltage spreading between two regulators, but that's not enough
> for the case of Tegra SoC's. It's a bit difficult to support universally
> all possible coupling restrictions in a form of device-tree description,
> so here comes the machine-specific coupling API which allow platforms
> to customize coupling algorithms.

Hello people,

I want to point out that the CPUFreq patches are currently blocked
because of the missing support for a regulators coupling on Tegra and we
want to switch to a proper-generic OPP voltage/freq API.

The other thing that I also forgot to mention that we will need a way to
keep on hold CORE voltage changes until all relevant peripheral drivers
are loaded and claimed the required voltage level. That is needed
because some of the critical peripherals are left in a running state
after bootloader. Currently, in this patchset, we are not allowing CORE
voltage to go lower than the level left after bootloader and once all
the relevant drivers will get support for the voltage management, we
should be able to unhold the lower CORE voltages around late_init().

Will be great if you could take a look at this series and tell your
opinion on the approach, I'll be happy to put more effort into it all if
will be needed. There is now a bit more advanced version of the series
that adds more error-checking and makes use of max-spread and max-step
values from the regular constraints (i.e. values defined in device-tree)
instead of hardcoding them in the code.

Re: [RFC PATCH v1 0/6] Introduce machine-specific regulators coupling API

[Mark Brown]

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

On Sun, May 05, 2019 at 05:57:42PM +0300, Dmitry Osipenko wrote:

> after bootloader. Currently, in this patchset, we are not allowing CORE
> voltage to go lower than the level left after bootloader and once all
> the relevant drivers will get support for the voltage management, we
> should be able to unhold the lower CORE voltages around late_init().

That's going to break as soon as someone like a distro builds drivers as
modules, you can't rely on things getting done at any particular point
in initialization or indeed on any given set of drivers being available
in the particular kernel that the user chooses to run - if they decide
not to build drivers for devices that they don't use on their particular
system that should work.

Overall this feels like an abstraction failure and you've not really
said what the constraints you're trying to implement here are so it's
hard to tell if that's the case or not.

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

Re: [RFC PATCH v1 0/6] Introduce machine-specific regulators coupling API

[Dmitry Osipenko]

08.05.2019 11:05, Mark Brown пишет:
> On Sun, May 05, 2019 at 05:57:42PM +0300, Dmitry Osipenko wrote:
> 
>> after bootloader. Currently, in this patchset, we are not allowing CORE
>> voltage to go lower than the level left after bootloader and once all
>> the relevant drivers will get support for the voltage management, we
>> should be able to unhold the lower CORE voltages around late_init().
> 
> That's going to break as soon as someone like a distro builds drivers as
> modules, you can't rely on things getting done at any particular point
> in initialization or indeed on any given set of drivers being available
> in the particular kernel that the user chooses to run - if they decide
> not to build drivers for devices that they don't use on their particular
> system that should work.

Yes, this is not really well-thought yet. Although I guess it should be
fine to assume that all the relevant hardware units are blocked by the
CLK framework that disables the orphaned clocks (by default).

Probably we'll have to better define what drivers are system-critical
for the platform, making sure that they are always included in the
kernel build and complied as built-in. But I also guess there could be
other (better) variants as well.

For now I'm primarily focused on getting the CPU voltage scaling to
work. The CORE minimum voltage is kept limited for now to the
boot-level. I'm assuming that the boot-stage voltages are at reasonable
levels for all boards because otherwise likely that there is a trouble
already (constraint violations).

> Overall this feels like an abstraction failure and you've not really
> said what the constraints you're trying to implement here are so it's
> hard to tell if that's the case or not.
> 

I described the coupling voltage constraints in the replies to the
relevant patches.

The CORE regulator supplies multiple peripherals in the SoC, each
peripheral has it's own demand for a minimum CORE voltage depending on a
clock rate and hardware version.

Tegra20 has the RTC (real-time-clock domain) regulator being coupled in
addition to the CORE. It doesn't supply any peripherals in the SoC that
have specific demands for minimum RTC voltage.

Please let me know if there is a need to explain anything else in a more
detail.