[RFC PATCH 0/3] regulator: bd718x7: support voltage scaling

[RFC PATCH 0/3] regulator: bd718x7: support voltage scaling

[Matti Vaittinen]

RFC for adding a support for typical voltage scaling connection

In few occasions there has been a need to scale the voltage output
from bucks on BD71837. Usually this is done when buck8 is used to
power specific GPU which can utilize voltages down to 0.7V. As lowest
the buck8 on BD71837 can go is 0.8V, and external connection is used to
scale the voltages.

The BD71837, BD71847 and BD71850 bucks can be adjusted by pulling up the
feedback pin using suitable voltage/resistors.

	|---------------|
	|       buck 8  |-------+----->Vout
	|               |       |
	|---------------|       |
	       |                |
	       |                |
	       +-------+--R2----+
	               |
	               R1
	               |
	       V FB-pull-up


This will scale the voltage as follows:
 - Vout_o = Vo - (Vpu - Vo)*R2/R1
 - Linear_step = step_orig*(R1+R2)/R1
where:
Vout_o is adjusted voltage output at vsel reg value 0
Vo is original voltage output at vsel reg value 0
Vpu is the pull-up voltage V FB-pull-up in the picture
R1 and R2 are resistor values.

From HW point of view this does not need to be limited to buck 8. This
connection can be used to adjust output from any of the bucks on
BD71837/47/50.

As this seems to be a 'de-facto' way to scale the voltages on BD71837 it
might be a good idea to support computing the new voltage ranges for
bucks based on the V-pull-up and resistor R1/R2 values given from
device-tree. This allows describing the external HW connection using DT
to correctly scale the voltages.

This RFC uses "rohm,feedback-pull-up-r1-ohms" and
"rohm,feedback-pull-up-r2-ohms" to provide the resistor values - but
these names (without the picture) might not be too descriptive. I am
grateful for all suggestions as better and more descriptive names.

This patch series is an RFC because this connection feels somewhat
"hacky". OTOH - when hack becomes widely used, it is less of an hack and
more of a standard - and occasionally supporting HW hacks using SW may
benefit us all, right? :)

The other thing some projects do is allowing the change of BD71837 buck8
voltages when buck8 is enabled. This however will introduce voltage
spikes as buck8 was not originally designed for this. The specific HW
platform must be evaluated to be able to tolerate these spikes. Thus
this patch series does not support buck8 voltage changes when buck8 is
enabled. I wonder if this should be allowed per some config option(?) I
don't want to help people frying their boards... Opinions? Is there
suggested way of allowing this type of features at own risk? Config or
even Some #ifdef which is not listed in Kconfig? Device-tree property?
 If you have (good) suggestions I could add the optional (non default)
DVS support for non DVS bucks on BD71837.


Matti Vaittinen (3):
  dt-bindings: regulator: BD71837 support commonly used feedback
    connection
  dt-bindings: regulator: BD71847 support commonly used feedback
    connection
  regulator: bd718x7: Support external connection to scale voltages

 .../regulator/rohm,bd71837-regulator.yaml     |  48 +++++
 .../regulator/rohm,bd71847-regulator.yaml     |  49 ++++++
 drivers/regulator/bd718x7-regulator.c         | 164 +++++++++++++++++-
 3 files changed, 254 insertions(+), 7 deletions(-)


base-commit: 3cea11cd5e3b00d91caf0b4730194039b45c5891
-- 
2.21.3


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

[RFC PATCH 1/3] dt-bindings: regulator: BD71837 support commonly used feedback connection

[Matti Vaittinen]

The BD71837 buck output voltages are in a few cases scaled using external
connection which adds a pull-up to regulator feedback pin. This connection
will adjust output voltage from regulator in a deterministic way.

Add support for describing this HW connection so that driver can adjust
voltage ranges accordingly.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 .../regulator/rohm,bd71837-regulator.yaml     | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/rohm,bd71837-regulator.yaml b/Documentation/devicetree/bindings/regulator/rohm,bd71837-regulator.yaml
index f5e31196a646..1941b36cf1ef 100644
--- a/Documentation/devicetree/bindings/regulator/rohm,bd71837-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/rohm,bd71837-regulator.yaml
@@ -105,6 +105,54 @@ patternProperties:
           PMIC hardware state machine.
         type: boolean
 
+      # Setups where regulator (especially the buck8) output voltage is scaled
+      # by adding external connection where some other regulator output is
+      # connected to feedback-pin (over suitable resistors) is getting popular
+      # amongst users of BD71837. (This allows for example scaling down the
+      # buck8 voltages to suit lover GPU voltages for projects where buck8 is
+      # (ab)used to supply power for GPU.
+      #
+      # So we allow describing this external connection from DT and scale the
+      # voltages accordingly. This is what the connection should look like:
+      #
+      # |---------------|
+      # |       buck 8  |-------+----->Vout
+      # |               |       |
+      # |---------------|       |
+      #        |                |
+      #        |                |
+      #        +-------+--R2----+
+      #                |
+      #                R1
+      #                |
+      #        V FB-pull-up
+      #
+      # Here the buck output is sifted according to formula:
+      #
+      # Vout_o = Vo - (Vpu - Vo)*R2/R1
+      # Linear_step = step_orig*(R1+R2)/R1
+      #
+      # where:
+      # Vout_o is adjusted voltage output at vsel reg value 0
+      # Vo is original voltage output at vsel reg value 0
+      # Vpu is the pull-up voltage V FB-pull-up in the picture
+      # R1 and R2 are resistor values.
+
+      rohm,fb-pull-up-microvolt:
+        description:
+          Feedback-pin has pull-up connection to adjust voltage range. This is
+          the used pull-up voltage before R1.
+
+      rohm,feedback-pull-up-r1-ohms:
+        description:
+          Feedback-pin has pull-up connection to adjust voltage range. This is
+          the used R1 resistor.
+
+      rohm,feedback-pull-up-r2-ohms:
+        description:
+          Feedback-pin has pull-up connection to adjust voltage range. This is
+          the used R2 resistor.
+
     required:
       - regulator-name
 
-- 
2.21.3


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

[RFC PATCH 2/3] dt-bindings: regulator: BD71847 support commonly used feedback connection

[Matti Vaittinen]

The BD71847 buck output voltages are in a few cases scaled using external
connection which adds a pull-up to regulator feedback pin. This connection
will adjust output voltage from regulator in a deterministic way.

Add support for describing this HW connection so that driver can adjust
voltage ranges accordingly.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 .../regulator/rohm,bd71847-regulator.yaml     | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/rohm,bd71847-regulator.yaml b/Documentation/devicetree/bindings/regulator/rohm,bd71847-regulator.yaml
index eeac32cd15d6..a1b806373853 100644
--- a/Documentation/devicetree/bindings/regulator/rohm,bd71847-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/rohm,bd71847-regulator.yaml
@@ -99,6 +99,55 @@ patternProperties:
           Enable/Disable control of this regulator must be left to the
           PMIC hardware state machine.
         type: boolean
+
+      # Setups where regulator (especially the buck8) output voltage is scaled
+      # by adding external connection where some other regulator output is
+      # connected to feedback-pin (over suitable resistors) is getting popular
+      # amongst users of BD71837. (This allows for example scaling down the
+      # buck8 voltages to suit lover GPU voltages for projects where buck8 is
+      # (ab)used to supply power for GPU.
+      #
+      # So we allow describing this external connection from DT and scale the
+      # voltages accordingly. This is what the connection should look like:
+      #
+      # |---------------|
+      # |       buck 8  |-------+----->Vout
+      # |               |       |
+      # |---------------|       |
+      #        |                |
+      #        |                |
+      #        +-------+--R2----+
+      #                |
+      #                R1
+      #                |
+      #        V FB-pull-up
+      #
+      # Here the buck output is sifted according to formula:
+      #
+      # Vout_o = Vo - (Vpu - Vo)*R2/R1
+      # Linear_step = step_orig*(R1+R2)/R1
+      #
+      # where:
+      # Vout_o is adjusted voltage output at vsel reg value 0
+      # Vo is original voltage output at vsel reg value 0
+      # Vpu is the pull-up voltage V FB-pull-up in the picture
+      # R1 and R2 are resistor values.
+
+      rohm,fb-pull-up-microvolt:
+        description:
+          Feedback-pin has pull-up connection to adjust voltage range. This is
+          the used pull-up voltage before R1.
+
+      rohm,feedback-pull-up-r1-ohms:
+        description:
+          Feedback-pin has pull-up connection to adjust voltage range. This is
+          the used R1 resistor.
+
+      rohm,feedback-pull-up-r2-ohms:
+        description:
+          Feedback-pin has pull-up connection to adjust voltage range. This is
+          the used R2 resistor.
+
     required:
       - regulator-name
 
-- 
2.21.3


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

[RFC PATCH 3/3] regulator: bd718x7: Support external connection to scale voltages

[Matti Vaittinen]

Setups where regulator (especially the buck8) output voltage is scaled
by adding external connection where some other regulator output is
connected to feedback-pin (over suitable resistors) is getting popular
amongst users of BD71837. This allows for example scaling down the
buck8 voltages to suit lover GPU voltages for projects where buck8 is
(ab)used to supply power for GPU. As a note - some setups do allow DVS
for buck8. This do produce voltage spikes and the HW must be evaluated
to be able to survive them. Thus this commit still keep the DVS disabled
for non DVS bucks by default. Let's not help you burn your proto board.

Allow describing this external connection from DT and scale the
voltages accordingly. This is what the connection should look like:

|------------|
|    buck 8  |-------+----->Vout
|            |       |
|------------|       |
     | FB pin        |
     |               |
     +-------+--R2---+
             |
             R1
             |
     V FB-pull-up

     Here the buck output is sifted according to formula:

Vout_o = Vo - (Vpu - Vo)*R2/R1
Linear_step = step_orig*(R1+R2)/R1

where:
Vout_o is adjusted voltage output at vsel reg value 0
Vo is original voltage output at vsel reg value 0
Vpu is the pull-up voltage V FB-pull-up in the picture
R1 and R2 are resistor values.

Bring support for specifying the Vpu, R1 and R2 from device tree and
scale voltages if they are given.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/regulator/bd718x7-regulator.c | 164 ++++++++++++++++++++++++--
 1 file changed, 157 insertions(+), 7 deletions(-)

diff --git a/drivers/regulator/bd718x7-regulator.c b/drivers/regulator/bd718x7-regulator.c
index 0774467994fb..e6d5d98c3cea 100644
--- a/drivers/regulator/bd718x7-regulator.c
+++ b/drivers/regulator/bd718x7-regulator.c
@@ -1323,13 +1323,142 @@ static void mark_hw_controlled(struct device *dev, struct device_node *np,
 	dev_warn(dev, "Bad regulator node\n");
 }
 
-static int get_hw_controlled_regulators(struct device *dev,
-					struct bd718xx_regulator_data *reg_data,
-					unsigned int num_reg_data, int *info)
+/*
+ * Setups where regulator (especially the buck8) output voltage is scaled
+ * by adding external connection where some other regulator output is connected
+ * to feedback-pin (over suitable resistors) is getting popular amongst users
+ * of BD71837. (This allows for example scaling down the buck8 voltages to suit
+ * lover GPU voltages for projects where buck8 is (ab)used to supply power
+ * for GPU. Additionally some setups do allow DVS for buck8 but as this do
+ * produce voltage spikes the HW must be evaluated to be able to survive this
+ * - hence I keep the DVS disabled for non DVS bucks by default. I don't want
+ * to help you burn your proto board)
+ *
+ * So we allow describing this external connection from DT and scale the
+ * voltages accordingly. This is what the connection should look like:
+ *
+ * |------------|
+ * |	buck 8  |-------+----->Vout
+ * |		|	|
+ * |------------|	|
+ *	| FB pin	|
+ *	|		|
+ *	+-------+--R2---+
+ *		|
+ *		R1
+ *		|
+ *	V FB-pull-up
+ *
+ *	Here the buck output is sifted according to formula:
+ *
+ * Vout_o = Vo - (Vpu - Vo)*R2/R1
+ * Linear_step = step_orig*(R1+R2)/R1
+ *
+ * where:
+ * Vout_o is adjusted voltage output at vsel reg value 0
+ * Vo is original voltage output at vsel reg value 0
+ * Vpu is the pull-up voltage V FB-pull-up in the picture
+ * R1 and R2 are resistor values.
+ *
+ * As a real world example for buck8 and a specific GPU:
+ * VLDO = 1.6V (used as FB-pull-up)
+ * R1 = 1000ohms
+ * R2 = 150ohms
+ * VSEL 0x0 => 0.8V – (VLDO – 0.8) * R2 / R1 = 0.68V
+ * Linear Step = 10mV * (R1 + R2) / R1 = 11.5mV
+ */
+static int setup_feedback_loop(struct device *dev, struct device_node *np,
+			       struct bd718xx_regulator_data *reg_data,
+			       unsigned int num_reg_data, int fb_uv)
 {
+	int i, r1, r2, ret;
+
+	/*
+	 * We do adjust the values in the global desc based on DT settings.
+	 * This may not be best approach as it can cause problems if more than
+	 * one PMIC is controlled from same processor. I don't see such use-case
+	 * for BD718x7 now - so we spare some bits.
+	 *
+	 * If this will point out to be a problem - then we can allocate new
+	 * bd718xx_regulator_data array at probe and just use the global
+	 * array as a template where we copy initial values. Then we can
+	 * use allocated descs for regultor registration and do IC specific
+	 * modifications to this copy while leaving other PMICs untouched. But
+	 * that means allocating new array for each PMIC - and currently I see
+	 * no need for that.
+	 */
+
+	for (i = 0; i < num_reg_data; i++) {
+		struct regulator_desc *desc = &reg_data[i].desc;
+		int j;
+
+		if (!of_node_name_eq(np, desc->of_match))
+			continue;
+
+		pr_info("Looking at node '%s'\n", desc->of_match);
+
+		/* The feedback loop connection does not make sense for LDOs */
+		if (desc->id >= BD718XX_LDO1)
+			return -EINVAL;
+
+		ret = of_property_read_u32(np, "rohm,feedback-pull-up-r1-ohms",
+					   &r1);
+		if (ret)
+			return ret;
+
+		if (!r1)
+			return -EINVAL;
+
+		ret = of_property_read_u32(np, "rohm,feedback-pull-up-r2-ohms",
+					   &r2);
+		if (ret)
+			return ret;
+
+		if (desc->n_linear_ranges && desc->linear_ranges) {
+			struct linear_range *new;
+
+			new = devm_kzalloc(dev, desc->n_linear_ranges *
+					   sizeof(struct linear_range),
+					   GFP_KERNEL);
+			if (!new)
+				return -ENOMEM;
+
+			for (j = 0; j < desc->n_linear_ranges; j++) {
+				int min = desc->linear_ranges[j].min;
+				int step = desc->linear_ranges[j].step;
+
+				min -= (fb_uv - min)*r2/r1;
+				step = step * (r1 + r2);
+				step /= r1;
+
+				new[j].min = min;
+				new[j].step = step;
+
+				dev_dbg(dev, "%s: old range min %d, step %d\n",
+					desc->name, desc->linear_ranges[j].min,
+					desc->linear_ranges[j].step);
+				dev_dbg(dev, "new range min %d, step %d\n", min,
+					step);
+			}
+			desc->linear_ranges = new;
+		}
+		dev_dbg(dev, "regulator '%s' has FB pull-up configured\n",
+			desc->name);
+
+		return 0;
+	}
+
+	return -ENODEV;
+}
+
+static int get_special_regulators(struct device *dev,
+				  struct bd718xx_regulator_data *reg_data,
+				  unsigned int num_reg_data, int *info)
+{
+	int ret;
 	struct device_node *np;
 	struct device_node *nproot = dev->of_node;
-	const char *prop = "rohm,no-regulator-enable-control";
+	int uv;
 
 	*info = 0;
 
@@ -1338,13 +1467,32 @@ static int get_hw_controlled_regulators(struct device *dev,
 		dev_err(dev, "failed to find regulators node\n");
 		return -ENODEV;
 	}
-	for_each_child_of_node(nproot, np)
-		if (of_property_read_bool(np, prop))
+	for_each_child_of_node(nproot, np) {
+		if (of_property_read_bool(np, "rohm,no-regulator-enable-control"))
 			mark_hw_controlled(dev, np, reg_data, num_reg_data,
 					   info);
+		ret = of_property_read_u32(np, "rohm,fb-pull-up-microvolt",
+					   &uv);
+		if (ret) {
+			if (ret == -EINVAL)
+				continue;
+			else
+				goto err_out;
+		}
+
+		ret = setup_feedback_loop(dev, np, reg_data, num_reg_data, uv);
+		if (ret)
+			goto err_out;
+	}
 
 	of_node_put(nproot);
 	return 0;
+
+err_out:
+	of_node_put(np);
+	of_node_put(nproot);
+
+	return ret;
 }
 
 static int bd718xx_probe(struct platform_device *pdev)
@@ -1432,8 +1580,10 @@ static int bd718xx_probe(struct platform_device *pdev)
 	 * be affected by PMIC state machine - Eg. regulator is likely to stay
 	 * on even in SUSPEND
 	 */
-	get_hw_controlled_regulators(pdev->dev.parent, reg_data, num_reg_data,
+	err = get_special_regulators(pdev->dev.parent, reg_data, num_reg_data,
 				     &omit_enable);
+	if (err)
+		return err;
 
 	for (i = 0; i < num_reg_data; i++) {
 
-- 
2.21.3


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

Re: [RFC PATCH 0/3] regulator: bd718x7: support voltage scaling

[Mark Brown]

On Tue, 10 Nov 2020 10:19:16 +0200, Matti Vaittinen wrote:
> RFC for adding a support for typical voltage scaling connection
> 
> In few occasions there has been a need to scale the voltage output
> from bucks on BD71837. Usually this is done when buck8 is used to
> power specific GPU which can utilize voltages down to 0.7V. As lowest
> the buck8 on BD71837 can go is 0.8V, and external connection is used to
> scale the voltages.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/3] regulator: BD71837 support commonly used feedback connection
      commit: 1e908b2419ea828dfad9819e5c01322a93665356
[2/3] regulator: BD71847 support commonly used feedback connection
      commit: b54a27d8109fc8f18cec3e0663f8e81ea727e3c6
[3/3] regulator: bd718x7: Support external connection to scale voltages
      commit: d2ad981151b3a812e961c8ee0ffd7e349b4027d6

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

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

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

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

Thanks,
Mark