diff mbox series

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

Message ID 20190414175939.12368-2-digetx@gmail.com
State Deferred
Headers show
Series Introduce machine-specific regulators coupling API | expand

Commit Message

Dmitry Osipenko April 14, 2019, 5:59 p.m. UTC
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(+)

Comments

Mark Brown May 8, 2019, 7:55 a.m. UTC | #1
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.
Dmitry Osipenko May 8, 2019, 1:05 p.m. UTC | #2
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.
diff mbox series

Patch

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