[PATCH 0/2] regulator: tps6586x: support for input supply

[PATCH 0/2] regulator: tps6586x: support for input supply

[Laxman Dewangan]

This patch series add the support for the input supply of tps6586x
regulator.
The changes are based on on discussion on patch
[PATCH 1/3] ARM: dt: tegra: seaboard: add regulators
on which the input supply name should be require in DT case and
unconditionally set on the desc.supply_name.

To add this, the existing user (non-dt) of this PMIC also need to fill
require input supply so that it will not break the functionality with
this support.
Currently harmony is using this PMIC and hence making the changes
on this as first change of series to avoid any functionality break.

The grep shows that there is no more user of this regulator and hence it
is safe to change in tps6586x regulator driver.

Laxman Dewangan (2):
  ARM: tegra: harmony: add regulator supply name and its input supply
  regulator: tps6586x: add support for input supply

 .../devicetree/bindings/regulator/tps6586x.txt     |   17 +++++++
 arch/arm/mach-tegra/board-harmony-power.c          |   33 ++++++++-----
 drivers/regulator/tps6586x-regulator.c             |   47 ++++++++++---------
 3 files changed, 62 insertions(+), 35 deletions(-)

[PATCH 1/2] ARM: tegra: harmony: add regulator supply name and its input supply

[Laxman Dewangan]

Name the regulator as per board schematics and adds its
supply name info in regulator data.
Add the always on fixed regulator to refer the battery supply.

Use this fixed regulator for input supply of some of PMIC
regulator

Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/mach-tegra/board-harmony-power.c |   33 +++++++++++++++++-----------
 1 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-tegra/board-harmony-power.c b/arch/arm/mach-tegra/board-harmony-power.c
index 44dcb2e..8fd387b 100644
--- a/arch/arm/mach-tegra/board-harmony-power.c
+++ b/arch/arm/mach-tegra/board-harmony-power.c
@@ -19,6 +19,7 @@
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
 #include <linux/regulator/machine.h>
+#include <linux/regulator/fixed.h>
 #include <linux/mfd/tps6586x.h>
 #include <linux/of.h>
 #include <linux/of_i2c.h>
@@ -34,7 +35,9 @@ static struct regulator_consumer_supply tps658621_ldo0_supply[] = {
 };
 
 static struct regulator_init_data ldo0_data = {
+	.supply_regulator = "vdd_sm2",
 	.constraints = {
+		.name = "vdd_ldo0",
 		.min_uV = 3300 * 1000,
 		.max_uV = 3300 * 1000,
 		.valid_modes_mask = (REGULATOR_MODE_NORMAL |
@@ -48,9 +51,11 @@ static struct regulator_init_data ldo0_data = {
 	.consumer_supplies = tps658621_ldo0_supply,
 };
 
-#define HARMONY_REGULATOR_INIT(_id, _minmv, _maxmv)			\
+#define HARMONY_REGULATOR_INIT(_id, _name, _supply, _minmv, _maxmv)	\
 	static struct regulator_init_data _id##_data = {		\
+		.supply_regulator = _supply,				\
 		.constraints = {					\
+			.name = _name,					\
 			.min_uV = (_minmv)*1000,			\
 			.max_uV = (_maxmv)*1000,			\
 			.valid_modes_mask = (REGULATOR_MODE_NORMAL |	\
@@ -61,18 +66,18 @@ static struct regulator_init_data ldo0_data = {
 		},							\
 	}
 
-HARMONY_REGULATOR_INIT(sm0, 725, 1500);
-HARMONY_REGULATOR_INIT(sm1, 725, 1500);
-HARMONY_REGULATOR_INIT(sm2, 3000, 4550);
-HARMONY_REGULATOR_INIT(ldo1, 725, 1500);
-HARMONY_REGULATOR_INIT(ldo2, 725, 1500);
-HARMONY_REGULATOR_INIT(ldo3, 1250, 3300);
-HARMONY_REGULATOR_INIT(ldo4, 1700, 2475);
-HARMONY_REGULATOR_INIT(ldo5, 1250, 3300);
-HARMONY_REGULATOR_INIT(ldo6, 1250, 3300);
-HARMONY_REGULATOR_INIT(ldo7, 1250, 3300);
-HARMONY_REGULATOR_INIT(ldo8, 1250, 3300);
-HARMONY_REGULATOR_INIT(ldo9, 1250, 3300);
+HARMONY_REGULATOR_INIT(sm0,  "vdd_sm0",  "vdd_sys", 725, 1500);
+HARMONY_REGULATOR_INIT(sm1,  "vdd_sm1",  "vdd_sys", 725, 1500);
+HARMONY_REGULATOR_INIT(sm2,  "vdd_sm2",  "vdd_sys", 3000, 4550);
+HARMONY_REGULATOR_INIT(ldo1, "vdd_ldo1", "vdd_sm2", 725, 1500);
+HARMONY_REGULATOR_INIT(ldo2, "vdd_ldo2", "vdd_sm2", 725, 1500);
+HARMONY_REGULATOR_INIT(ldo3, "vdd_ldo3", "vdd_sm2", 1250, 3300);
+HARMONY_REGULATOR_INIT(ldo4, "vdd_ldo4", "vdd_sm2", 1700, 2475);
+HARMONY_REGULATOR_INIT(ldo5, "vdd_ldo5", NULL,	    1250, 3300);
+HARMONY_REGULATOR_INIT(ldo6, "vdd_ldo6", "vdd_sm2", 1250, 3300);
+HARMONY_REGULATOR_INIT(ldo7, "vdd_ldo7", "vdd_sm2", 1250, 3300);
+HARMONY_REGULATOR_INIT(ldo8, "vdd_ldo8", "vdd_sm2", 1250, 3300);
+HARMONY_REGULATOR_INIT(ldo9, "vdd_ldo9", "vdd_sm2", 1250, 3300);
 
 #define TPS_REG(_id, _data)			\
 	{					\
@@ -115,6 +120,8 @@ static struct i2c_board_info __initdata harmony_regulators[] = {
 int __init harmony_regulator_init(void)
 {
 	if (machine_is_harmony()) {
+		regulator_register_always_on(0, "vdd_sys",
+			NULL, 0, 5000000);
 		i2c_register_board_info(3, harmony_regulators, 1);
 	} else { /* Harmony, booted using device tree */
 		struct device_node *np;
-- 
1.7.1.1

Re: [PATCH 1/2] ARM: tegra: harmony: add regulator supply name and its input supply

[Stephen Warren]

On 07/13/2012 07:50 AM, Laxman Dewangan wrote:
> Name the regulator as per board schematics and adds its
> supply name info in regulator data.
> Add the always on fixed regulator to refer the battery supply.
> 
> Use this fixed regulator for input supply of some of PMIC
> regulator
> 
> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Acked-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

Note that this file might well be deleted within the 3.7 development
cycle, since we will presumably get to the point where when booting from
DT, the DT will set up the regulators, and we won't support non-DT
booting any more on Harmony. Still, that should merge without issues, so
I don't think this will be a problem.

[PATCH 2/2] regulator: tps6586x: add support for input supply

[Laxman Dewangan]

There is multiple voltage input pins on device which
takes the voltage input for different voltage regulator.
Support to configure the voltage input supplied by
different regulator for each regulators.

Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 .../devicetree/bindings/regulator/tps6586x.txt     |   17 +++++++
 drivers/regulator/tps6586x-regulator.c             |   47 ++++++++++---------
 2 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/tps6586x.txt b/Documentation/devicetree/bindings/regulator/tps6586x.txt
index ab17ef6..d156e1b 100644
--- a/Documentation/devicetree/bindings/regulator/tps6586x.txt
+++ b/Documentation/devicetree/bindings/regulator/tps6586x.txt
@@ -9,6 +9,14 @@ Required properties:
 - regulators: list of regulators provided by this controller, must have
   property "regulator-compatible" to match their hardware counterparts:
   sm[0-2], ldo[0-9] and ldo_rtc
+- sm0-supply: The input supply for the SM0.
+- sm1-supply: The input supply for the SM1.
+- sm2-supply: The input supply for the SM2.
+- vinldo01-supply: The input supply for the LDO1 and LDO2
+- vinldo23-supply: The input supply for the LDO2 and LDO3
+- vinldo4-supply: The input supply for the LDO4
+- vinldo678-supply: The input supply for the LDO6, LDO7 and LDO8
+- vinldo9-supply: The input supply for the LDO9
 
 Each regulator is defined using the standard binding for regulators.
 
@@ -22,6 +30,15 @@ Example:
 		#gpio-cells = <2>;
 		gpio-controller;
 
+		sm0-supply = <&some_reg>;
+		sm1-supply = <&some_reg>;
+		sm2-supply = <&some_reg>;
+		vinldo01-supply = <...>;
+		vinldo23-supply = <...>;
+		vinldo4-supply = <...>;
+		vinldo678-supply = <...>;
+		vinldo9-supply = <...>;
+
 		regulators {
 			#address-cells = <1>;
 			#size-cells = <0>;
diff --git a/drivers/regulator/tps6586x-regulator.c b/drivers/regulator/tps6586x-regulator.c
index c0ac241..e6da90a 100644
--- a/drivers/regulator/tps6586x-regulator.c
+++ b/drivers/regulator/tps6586x-regulator.c
@@ -70,7 +70,7 @@ struct tps6586x_regulator {
 
 static inline struct device *to_tps6586x_dev(struct regulator_dev *rdev)
 {
-	return rdev_get_dev(rdev)->parent->parent;
+	return rdev_get_dev(rdev)->parent;
 }
 
 static int tps6586x_set_voltage_sel(struct regulator_dev *rdev,
@@ -191,9 +191,10 @@ static const unsigned int tps6586x_dvm_voltages[] = {
 	1325000, 1350000, 1375000, 1400000, 1425000, 1450000, 1475000, 1500000,
 };
 
-#define TPS6586X_REGULATOR(_id, vdata, vreg, shift, nbits,		\
+#define TPS6586X_REGULATOR(_id, _pin_name, vdata, vreg, shift, nbits,	\
 			   ereg0, ebit0, ereg1, ebit1)			\
 	.desc	= {							\
+		.supply_name = _pin_name,				\
 		.name	= "REG-" #_id,					\
 		.ops	= &tps6586x_regulator_ops,			\
 		.type	= REGULATOR_VOLTAGE,				\
@@ -214,37 +215,39 @@ static const unsigned int tps6586x_dvm_voltages[] = {
 	.go_reg = TPS6586X_##goreg,					\
 	.go_bit = (gobit),
 
-#define TPS6586X_LDO(_id, vdata, vreg, shift, nbits,			\
+#define TPS6586X_LDO(_id, _pname, vdata, vreg, shift, nbits,		\
 		     ereg0, ebit0, ereg1, ebit1)			\
 {									\
-	TPS6586X_REGULATOR(_id, vdata, vreg, shift, nbits,		\
+	TPS6586X_REGULATOR(_id, _pname, vdata, vreg, shift, nbits,	\
 			   ereg0, ebit0, ereg1, ebit1)			\
 }
 
-#define TPS6586X_DVM(_id, vdata, vreg, shift, nbits,			\
+#define TPS6586X_DVM(_id, _pname, vdata, vreg, shift, nbits,		\
 		     ereg0, ebit0, ereg1, ebit1, goreg, gobit)		\
 {									\
-	TPS6586X_REGULATOR(_id, vdata, vreg, shift, nbits,		\
+	TPS6586X_REGULATOR(_id, _pname, vdata, vreg, shift, nbits,	\
 			   ereg0, ebit0, ereg1, ebit1)			\
 	TPS6586X_REGULATOR_DVM_GOREG(goreg, gobit)			\
 }
 
 static struct tps6586x_regulator tps6586x_regulator[] = {
-	TPS6586X_LDO(LDO_0, ldo0, SUPPLYV1, 5, 3, ENC, 0, END, 0),
-	TPS6586X_LDO(LDO_3, ldo, SUPPLYV4, 0, 3, ENC, 2, END, 2),
-	TPS6586X_LDO(LDO_5, ldo, SUPPLYV6, 0, 3, ENE, 6, ENE, 6),
-	TPS6586X_LDO(LDO_6, ldo, SUPPLYV3, 0, 3, ENC, 4, END, 4),
-	TPS6586X_LDO(LDO_7, ldo, SUPPLYV3, 3, 3, ENC, 5, END, 5),
-	TPS6586X_LDO(LDO_8, ldo, SUPPLYV2, 5, 3, ENC, 6, END, 6),
-	TPS6586X_LDO(LDO_9, ldo, SUPPLYV6, 3, 3, ENE, 7, ENE, 7),
-	TPS6586X_LDO(LDO_RTC, ldo, SUPPLYV4, 3, 3, V4, 7, V4, 7),
-	TPS6586X_LDO(LDO_1, dvm, SUPPLYV1, 0, 5, ENC, 1, END, 1),
-	TPS6586X_LDO(SM_2, sm2, SUPPLYV2, 0, 5, ENC, 7, END, 7),
-
-	TPS6586X_DVM(LDO_2, dvm, LDO2BV1, 0, 5, ENA, 3, ENB, 3, VCC2, 6),
-	TPS6586X_DVM(LDO_4, ldo4, LDO4V1, 0, 5, ENC, 3, END, 3, VCC1, 6),
-	TPS6586X_DVM(SM_0, dvm, SM0V1, 0, 5, ENA, 1, ENB, 1, VCC1, 2),
-	TPS6586X_DVM(SM_1, dvm, SM1V1, 0, 5, ENA, 0, ENB, 0, VCC1, 0),
+	TPS6586X_LDO(LDO_0, "vinldo01", ldo0, SUPPLYV1, 5, 3, ENC, 0, END, 0),
+	TPS6586X_LDO(LDO_3, "vinldo23", ldo, SUPPLYV4, 0, 3, ENC, 2, END, 2),
+	TPS6586X_LDO(LDO_5, NULL, ldo, SUPPLYV6, 0, 3, ENE, 6, ENE, 6),
+	TPS6586X_LDO(LDO_6, "vinldo678", ldo, SUPPLYV3, 0, 3, ENC, 4, END, 4),
+	TPS6586X_LDO(LDO_7, "vinldo678", ldo, SUPPLYV3, 3, 3, ENC, 5, END, 5),
+	TPS6586X_LDO(LDO_8, "vinldo678", ldo, SUPPLYV2, 5, 3, ENC, 6, END, 6),
+	TPS6586X_LDO(LDO_9, "vinldo9", ldo, SUPPLYV6, 3, 3, ENE, 7, ENE, 7),
+	TPS6586X_LDO(LDO_RTC, NULL, ldo, SUPPLYV4, 3, 3, V4, 7, V4, 7),
+	TPS6586X_LDO(LDO_1, "vinldo01", dvm, SUPPLYV1, 0, 5, ENC, 1, END, 1),
+	TPS6586X_LDO(SM_2, "sm2", sm2, SUPPLYV2, 0, 5, ENC, 7, END, 7),
+
+	TPS6586X_DVM(LDO_2, "vinldo23", dvm, LDO2BV1, 0, 5, ENA, 3,
+					ENB, 3, VCC2, 6),
+	TPS6586X_DVM(LDO_4, "vinldo4", ldo4, LDO4V1, 0, 5, ENC, 3,
+					END, 3, VCC1, 6),
+	TPS6586X_DVM(SM_0, "sm0", dvm, SM0V1, 0, 5, ENA, 1, ENB, 1, VCC1, 2),
+	TPS6586X_DVM(SM_1, "sm1", dvm, SM1V1, 0, 5, ENA, 0, ENB, 0, VCC1, 0),
 };
 
 /*
@@ -351,7 +354,7 @@ static int __devinit tps6586x_regulator_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
-	config.dev = &pdev->dev;
+	config.dev = pdev->dev.parent;
 	config.of_node = pdev->dev.of_node;
 	config.init_data = pdev->dev.platform_data;
 	config.driver_data = ri;
-- 
1.7.1.1

Re: [PATCH 2/2] regulator: tps6586x: add support for input supply

[Mark Brown]

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

On Fri, Jul 13, 2012 at 07:20:32PM +0530, Laxman Dewangan wrote:
> There is multiple voltage input pins on device which
> takes the voltage input for different voltage regulator.
> Support to configure the voltage input supplied by
> different regulator for each regulators.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] regulator: tps6586x: add support for input supply

[Laxman Dewangan]

On Friday 13 July 2012 07:52 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Fri, Jul 13, 2012 at 07:20:32PM +0530, Laxman Dewangan wrote:
>> There is multiple voltage input pins on device which
>> takes the voltage input for different voltage regulator.
>> Support to configure the voltage input supplied by
>> different regulator for each regulators.
> Applied, thanks.
Thanks for care.

I was thinking that 1/2 should also go through your tree to avoid any 
functionality break on existing platform who are using this regulator.

Re: [PATCH 2/2] regulator: tps6586x: add support for input supply

[Mark Brown]

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

On Fri, Jul 13, 2012 at 10:45:12PM +0530, Laxman Dewangan wrote:

> I was thinking that 1/2 should also go through your tree to avoid
> any functionality break on existing platform who are using this
> regulator.

I'd be happy to do that if the ARM people are happy with it; just let me
know and resend the patch if you want me to apply it.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] regulator: tps6586x: add support for input supply

[Stephen Warren]

On 07/15/2012 03:21 PM, Mark Brown wrote:
> On Fri, Jul 13, 2012 at 10:45:12PM +0530, Laxman Dewangan wrote:
> 
>> I was thinking that 1/2 should also go through your tree to
>> avoid any functionality break on existing platform who are using
>> this regulator.
> 
> I'd be happy to do that if the ARM people are happy with it; just
> let me know and resend the patch if you want me to apply it.

Patch 2/2 really depends on patch 1/2 to avoid git bisect breakage;
are you able to rebase it to maintain that order when applying it?

Re: [PATCH 2/2] regulator: tps6586x: add support for input supply

[Mark Brown]

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

On Mon, Jul 23, 2012 at 02:54:55PM -0600, Stephen Warren wrote:
> On 07/15/2012 03:21 PM, Mark Brown wrote:
> > On Fri, Jul 13, 2012 at 10:45:12PM +0530, Laxman Dewangan wrote:

> >> I was thinking that 1/2 should also go through your tree to
> >> avoid any functionality break on existing platform who are using
> >> this regulator.

> > I'd be happy to do that if the ARM people are happy with it; just
> > let me know and resend the patch if you want me to apply it.

> Patch 2/2 really depends on patch 1/2 to avoid git bisect breakage;
> are you able to rebase it to maintain that order when applying it?

No, especially since Linus already merged the regulator changes for 3.6.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] regulator: tps6586x: add support for input supply

[Stephen Warren]

On 07/13/2012 07:50 AM, Laxman Dewangan wrote:
> There is multiple voltage input pins on device which
> takes the voltage input for different voltage regulator.
> Support to configure the voltage input supplied by
> different regulator for each regulators.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
>  .../devicetree/bindings/regulator/tps6586x.txt     |   17 +++++++
>  drivers/regulator/tps6586x-regulator.c             |   47 ++++++++++---------
>  2 files changed, 42 insertions(+), 22 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/tps6586x.txt b/Documentation/devicetree/bindings/regulator/tps6586x.txt
> index ab17ef6..d156e1b 100644
> --- a/Documentation/devicetree/bindings/regulator/tps6586x.txt
> +++ b/Documentation/devicetree/bindings/regulator/tps6586x.txt
> @@ -9,6 +9,14 @@ Required properties:
>  - regulators: list of regulators provided by this controller, must have
>    property "regulator-compatible" to match their hardware counterparts:
>    sm[0-2], ldo[0-9] and ldo_rtc
> +- sm0-supply: The input supply for the SM0.
> +- sm1-supply: The input supply for the SM1.
> +- sm2-supply: The input supply for the SM2.
> +- vinldo01-supply: The input supply for the LDO1 and LDO2
> +- vinldo23-supply: The input supply for the LDO2 and LDO3
> +- vinldo4-supply: The input supply for the LDO4
> +- vinldo678-supply: The input supply for the LDO6, LDO7 and LDO8
> +- vinldo9-supply: The input supply for the LDO9

Hmm. The signal names in my data sheet are VIN_SMn and VINLDOn, so
having "vin" in just some of the property names seems a little inconsistent.

> diff --git a/drivers/regulator/tps6586x-regulator.c b/drivers/regulator/tps6586x-regulator.c

>  static inline struct device *to_tps6586x_dev(struct regulator_dev *rdev)
>  {
> -	return rdev_get_dev(rdev)->parent->parent;
> +	return rdev_get_dev(rdev)->parent;
...
> @@ -351,7 +354,7 @@ static int __devinit tps6586x_regulator_probe(struct platform_device *pdev)
...
> -	config.dev = &pdev->dev;
> +	config.dev = pdev->dev.parent;

Those seem like unrelated changes, but I suppose it's fine.

Re: [PATCH 2/2] regulator: tps6586x: add support for input supply

[Mark Brown]

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

On Mon, Jul 23, 2012 at 02:51:44PM -0600, Stephen Warren wrote:
> On 07/13/2012 07:50 AM, Laxman Dewangan wrote:

> > -	config.dev = &pdev->dev;
> > +	config.dev = pdev->dev.parent;

> Those seem like unrelated changes, but I suppose it's fine.

Not entirely, it's being done so that the supplies are on the I2C device
node rather than on the synthetic node that we generated to instantiate
the regulator driver from our MFD.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] regulator: tps6586x: add support for input supply

[Laxman Dewangan]

On Tuesday 24 July 2012 02:21 AM, Stephen Warren wrote:
> On 07/13/2012 07:50 AM, Laxman Dewangan wrote:
>>     sm[0-2], ldo[0-9] and ldo_rtc
>> +- sm0-supply: The input supply for the SM0.
>> +- sm1-supply: The input supply for the SM1.
>> +- sm2-supply: The input supply for the SM2.
>> +- vinldo01-supply: The input supply for the LDO1 and LDO2
>> +- vinldo23-supply: The input supply for the LDO2 and LDO3
>> +- vinldo4-supply: The input supply for the LDO4
>> +- vinldo678-supply: The input supply for the LDO6, LDO7 and LDO8
>> +- vinldo9-supply: The input supply for the LDO9
> Hmm. The signal names in my data sheet are VIN_SMn and VINLDOn, so
> having "vin" in just some of the property names seems a little inconsistent.
>

My bad, not sure why I missed it.  I will send the patch for correcting 
this before Tegra's board ventana/harmony fills the dt entry.