[PATCH V3 0/4] regulator: tps62360: support dts and enable for tegra-cardhu

[PATCH V3 0/4] regulator: tps62360: support dts and enable for tegra-cardhu

[Laxman Dewangan]

Add support for dt on regulator device tps62360. Also  enable this regulator for Nvidia's
Tegra platform - cardhu.
This series:
- Make the regulator_init_data as pointer type from platform data.
- enable config variable in tegra defconfig
- add entry for tps62360 in dts file of tegra-cardhu.

Changes from V1:
	Patch 1 did not compile when not defining CONFIG_OF. Fixed it.

Changes from V2:
	Taken care of Grant's review comments.

Laxman Dewangan (4):
  regulator: tps62360: make init_data of platform data to pointer.
	Make the regulator init data as pointer type in platform.
  regulator: tps62360: add dt support
	Ad dt support.
  ARM: tegra: config: enable REGULATOR_TPS62360
  ARM: dt: tegra: cardhu: register core regulator tps62360

 .../bindings/regulator/tps62360-regulator.txt      |   45 ++++++++++++
 arch/arm/boot/dts/tegra-cardhu.dts                 |   14 ++++
 arch/arm/configs/tegra_defconfig                   |    1 +
 drivers/regulator/tps62360-regulator.c             |   74 +++++++++++++++++++-
 include/linux/regulator/tps62360.h                 |    4 +-
 5 files changed, 133 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/tps62360-regulator.txt

[PATCH V3 0/4] regulator: tps62360: support dts and enable for tegra-cardhu

[Laxman Dewangan]

Add support for dt on regulator device tps62360. Also  enable this regulator for Nvidia's
Tegra platform - cardhu.
This series:
- Make the regulator_init_data as pointer type from platform data.
- enable config variable in tegra defconfig
- add entry for tps62360 in dts file of tegra-cardhu.

Changes from V1:
	Patch 1 did not compile when not defining CONFIG_OF. Fixed it.

Changes from V2:
	Taken care of Grant's review comments.

Laxman Dewangan (4):
  regulator: tps62360: make init_data of platform data to pointer.
	Make the regulator init data as pointer type in platform.
  regulator: tps62360: add dt support
	Ad dt support.
  ARM: tegra: config: enable REGULATOR_TPS62360
  ARM: dt: tegra: cardhu: register core regulator tps62360

 .../bindings/regulator/tps62360-regulator.txt      |   45 ++++++++++++
 arch/arm/boot/dts/tegra-cardhu.dts                 |   14 ++++
 arch/arm/configs/tegra_defconfig                   |    1 +
 drivers/regulator/tps62360-regulator.c             |   74 +++++++++++++++++++-
 include/linux/regulator/tps62360.h                 |    4 +-
 5 files changed, 133 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/tps62360-regulator.txt

[PATCH V3 1/4] regulator: tps62360: make init_data of platform data to pointer.

[Laxman Dewangan]

Convert platform data member regulator_init_data to pointer type.
This will avoid the copy of entire regualator init data into
platform data member when adding dt support and it can be achieve
by simple assignment:
pdata->init_data = of_get_regulator_init_data(dev, dev->of_node);

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
No Changes from V1 and V2.

 drivers/regulator/tps62360-regulator.c |    2 +-
 include/linux/regulator/tps62360.h     |    4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/tps62360-regulator.c b/drivers/regulator/tps62360-regulator.c
index 97fd98b..3506900 100644
--- a/drivers/regulator/tps62360-regulator.c
+++ b/drivers/regulator/tps62360-regulator.c
@@ -408,7 +408,7 @@ static int __devinit tps62360_probe(struct i2c_client *client,
 	}
 
 	config.dev = &client->dev;
-	config.init_data = &pdata->reg_init_data;
+	config.init_data = pdata->reg_init_data;
 	config.driver_data = tps;
 
 	/* Register the regulators */
diff --git a/include/linux/regulator/tps62360.h b/include/linux/regulator/tps62360.h
index 6a5c1b2..5e11d50 100644
--- a/include/linux/regulator/tps62360.h
+++ b/include/linux/regulator/tps62360.h
@@ -26,8 +26,6 @@
 #ifndef __LINUX_REGULATOR_TPS62360_H
 #define __LINUX_REGULATOR_TPS62360_H
 
-#include <linux/regulator/machine.h>
-
 /*
  * struct tps62360_regulator_platform_data - tps62360 regulator platform data.
  *
@@ -44,7 +42,7 @@
  * @vsel1_def_state: Default state of vsel1. 1 if it is high else 0.
  */
 struct tps62360_regulator_platform_data {
-	struct regulator_init_data reg_init_data;
+	struct regulator_init_data *reg_init_data;
 	bool en_force_pwm;
 	bool en_discharge;
 	bool en_internal_pulldn;
-- 
1.7.1.1

[PATCH V3 1/4] regulator: tps62360: make init_data of platform data to pointer.

[Laxman Dewangan]

Convert platform data member regulator_init_data to pointer type.
This will avoid the copy of entire regualator init data into
platform data member when adding dt support and it can be achieve
by simple assignment:
pdata->init_data = of_get_regulator_init_data(dev, dev->of_node);

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
No Changes from V1 and V2.

 drivers/regulator/tps62360-regulator.c |    2 +-
 include/linux/regulator/tps62360.h     |    4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/tps62360-regulator.c b/drivers/regulator/tps62360-regulator.c
index 97fd98b..3506900 100644
--- a/drivers/regulator/tps62360-regulator.c
+++ b/drivers/regulator/tps62360-regulator.c
@@ -408,7 +408,7 @@ static int __devinit tps62360_probe(struct i2c_client *client,
 	}
 
 	config.dev = &client->dev;
-	config.init_data = &pdata->reg_init_data;
+	config.init_data = pdata->reg_init_data;
 	config.driver_data = tps;
 
 	/* Register the regulators */
diff --git a/include/linux/regulator/tps62360.h b/include/linux/regulator/tps62360.h
index 6a5c1b2..5e11d50 100644
--- a/include/linux/regulator/tps62360.h
+++ b/include/linux/regulator/tps62360.h
@@ -26,8 +26,6 @@
 #ifndef __LINUX_REGULATOR_TPS62360_H
 #define __LINUX_REGULATOR_TPS62360_H
 
-#include <linux/regulator/machine.h>
-
 /*
  * struct tps62360_regulator_platform_data - tps62360 regulator platform data.
  *
@@ -44,7 +42,7 @@
  * @vsel1_def_state: Default state of vsel1. 1 if it is high else 0.
  */
 struct tps62360_regulator_platform_data {
-	struct regulator_init_data reg_init_data;
+	struct regulator_init_data *reg_init_data;
 	bool en_force_pwm;
 	bool en_discharge;
 	bool en_internal_pulldn;
-- 
1.7.1.1

Re: [PATCH V3 1/4] regulator: tps62360: make init_data of platform data to pointer.

[Mark Brown]

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

On Fri, May 11, 2012 at 12:08:42PM +0530, Laxman Dewangan wrote:
> Convert platform data member regulator_init_data to pointer type.
> This will avoid the copy of entire regualator init data into
> platform data member when adding dt support and it can be achieve
> by simple assignment:
> pdata->init_data = of_get_regulator_init_data(dev, dev->of_node);

Applied, thanks.

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

[PATCH V3 2/4] regulator: tps62360: add dt support

[Laxman Dewangan]

Add dt support for the pmu device tps62360 and
Add binding documentation with example.
With this patch driver will support both device-tree and
non-device tree registration.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
Changes from V1:
	Fixed compilation error in non-devicetree configuration.
Changes from V2:
	Taken care of Grant's review comments.

 .../bindings/regulator/tps62360-regulator.txt      |   45 ++++++++++++
 drivers/regulator/tps62360-regulator.c             |   73 +++++++++++++++++++-
 2 files changed, 117 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/tps62360-regulator.txt

diff --git a/Documentation/devicetree/bindings/regulator/tps62360-regulator.txt b/Documentation/devicetree/bindings/regulator/tps62360-regulator.txt
new file mode 100644
index 0000000..f411b57
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/tps62360-regulator.txt
@@ -0,0 +1,45 @@
+TPS62360 Voltage regulators
+
+Required properties:
+- compatible: Must be one of the following.
+	"ti,tps62360"
+	"ti,tps62361",
+	"ti,tps62362",
+	"ti,tps62363",
+- reg: I2C slave address
+
+Optional properties:
+- ti,enable-force-pwm: Enable force PWM mode. This is boolean value.
+- ti,enable-vout-discharge: Enable output discharge. This is boolean value.
+- ti,enable-pull-down: Enable pull down. This is boolean value.
+- ti,vsel0-gpio: GPIO for controlling VSEL0 line.
+  If this property is missing, then assume that there is no GPIO
+  for vsel0 control.
+- ti,vsel1-gpio: Gpio for controlling VSEL1 line.
+  If this property is missing, then assume that there is no GPIO
+  for vsel1 control.
+- ti,vsel0-state-high: Inital state of vsel0 input is high.
+  If this property is missing, then assume the state as low (0).
+- ti,vsel1-state-high: Inital state of vsel1 input is high.
+  If this property is missing, then assume the state as low (0).
+
+Any property defined as part of the core regulator binding, defined in
+regulator.txt, can also be used.
+
+Example:
+
+	abc: tps62360 {
+		compatible = "ti,tps62361";
+		reg =  <0x60>;
+		regulator-name = "tps62361-vout";
+		regulator-min-microvolt = <500000>;
+		regulator-max-microvolt = <1500000>;
+		regulator-boot-on
+		ti,vsel0-gpio = <&gpio1 16 0>;
+		ti,vsel1-gpio = <&gpio1 17 0>;
+		ti,vsel0-state-high;
+		ti,vsel1-state-high;
+		ti,enable-pull-down;
+		ti,enable-force-pwm;
+		ti,enable-vout-discharge;
+	};
diff --git a/drivers/regulator/tps62360-regulator.c b/drivers/regulator/tps62360-regulator.c
index 3506900..60765cc 100644
--- a/drivers/regulator/tps62360-regulator.c
+++ b/drivers/regulator/tps62360-regulator.c
@@ -26,6 +26,10 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/regulator/of_regulator.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
@@ -297,6 +301,56 @@ static const struct regmap_config tps62360_regmap_config = {
 	.cache_type		= REGCACHE_RBTREE,
 };
 
+static struct tps62360_regulator_platform_data *
+	of_get_tps62360_platform_data(struct device *dev)
+{
+	struct tps62360_regulator_platform_data *pdata;
+	struct device_node *np = dev->of_node;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(dev, "Memory alloc failed for platform data\n");
+		return NULL;
+	}
+
+	pdata->reg_init_data = of_get_regulator_init_data(dev, dev->of_node);
+	if (!pdata->reg_init_data) {
+		dev_err(dev, "Not able to get OF regulator init data\n");
+		return NULL;
+	}
+
+	pdata->vsel0_gpio = of_get_named_gpio(np, "vsel0-gpio", 0);
+	pdata->vsel1_gpio = of_get_named_gpio(np, "vsel1-gpio", 0);
+
+	if (of_find_property(np, "ti,vsel0-state-high", NULL))
+		pdata->vsel0_def_state = 1;
+
+	if (of_find_property(np, "ti,vsel1-state-high", NULL))
+		pdata->vsel1_def_state = 1;
+
+	if (of_find_property(np, "ti,enable-pull-down", NULL))
+		pdata->en_internal_pulldn = true;
+
+	if (of_find_property(np, "ti,enable-force-pwm", NULL))
+		pdata->en_force_pwm = true;
+
+	if (of_find_property(np, "ti,enable-vout-discharge", NULL))
+		pdata->en_discharge = true;
+
+	return pdata;
+}
+
+#if defined(CONFIG_OF)
+static const struct of_device_id tps62360_of_match[] = {
+	 { .compatible = "ti,tps62360", .data = (void *)TPS62360},
+	 { .compatible = "ti,tps62361", .data = (void *)TPS62361},
+	 { .compatible = "ti,tps62362", .data = (void *)TPS62362},
+	 { .compatible = "ti,tps62363", .data = (void *)TPS62363},
+	{},
+}
+MODULE_DEVICE_TABLE(of, tps62360_of_match);
+#endif
+
 static int __devinit tps62360_probe(struct i2c_client *client,
 				     const struct i2c_device_id *id)
 {
@@ -306,8 +360,24 @@ static int __devinit tps62360_probe(struct i2c_client *client,
 	struct tps62360_chip *tps;
 	int ret;
 	int i;
+	int chip_id;
 
 	pdata = client->dev.platform_data;
+	chip_id = id->driver_data;
+
+	if (client->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_device(of_match_ptr(tps62360_of_match),
+				&client->dev);
+		if (!match) {
+			dev_err(&client->dev, "Error: No device match found\n");
+			return -ENODEV;
+		}
+		chip_id = (int)match->data;
+		if (!pdata)
+			pdata = of_get_tps62360_platform_data(&client->dev);
+	}
+
 	if (!pdata) {
 		dev_err(&client->dev, "%s(): Platform data not found\n",
 						__func__);
@@ -328,7 +398,7 @@ static int __devinit tps62360_probe(struct i2c_client *client,
 	tps->vsel1_gpio = pdata->vsel1_gpio;
 	tps->dev = &client->dev;
 
-	switch (id->driver_data) {
+	switch (chip_id) {
 	case TPS62360:
 	case TPS62362:
 		tps->voltage_base = TPS62360_BASE_VOLTAGE;
@@ -484,6 +554,7 @@ static struct i2c_driver tps62360_i2c_driver = {
 	.driver = {
 		.name = "tps62360",
 		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(tps62360_of_match),
 	},
 	.probe = tps62360_probe,
 	.remove = __devexit_p(tps62360_remove),
-- 
1.7.1.1

[PATCH V3 2/4] regulator: tps62360: add dt support

[Laxman Dewangan]

Add dt support for the pmu device tps62360 and
Add binding documentation with example.
With this patch driver will support both device-tree and
non-device tree registration.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
Changes from V1:
	Fixed compilation error in non-devicetree configuration.
Changes from V2:
	Taken care of Grant's review comments.

 .../bindings/regulator/tps62360-regulator.txt      |   45 ++++++++++++
 drivers/regulator/tps62360-regulator.c             |   73 +++++++++++++++++++-
 2 files changed, 117 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/tps62360-regulator.txt

diff --git a/Documentation/devicetree/bindings/regulator/tps62360-regulator.txt b/Documentation/devicetree/bindings/regulator/tps62360-regulator.txt
new file mode 100644
index 0000000..f411b57
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/tps62360-regulator.txt
@@ -0,0 +1,45 @@
+TPS62360 Voltage regulators
+
+Required properties:
+- compatible: Must be one of the following.
+	"ti,tps62360"
+	"ti,tps62361",
+	"ti,tps62362",
+	"ti,tps62363",
+- reg: I2C slave address
+
+Optional properties:
+- ti,enable-force-pwm: Enable force PWM mode. This is boolean value.
+- ti,enable-vout-discharge: Enable output discharge. This is boolean value.
+- ti,enable-pull-down: Enable pull down. This is boolean value.
+- ti,vsel0-gpio: GPIO for controlling VSEL0 line.
+  If this property is missing, then assume that there is no GPIO
+  for vsel0 control.
+- ti,vsel1-gpio: Gpio for controlling VSEL1 line.
+  If this property is missing, then assume that there is no GPIO
+  for vsel1 control.
+- ti,vsel0-state-high: Inital state of vsel0 input is high.
+  If this property is missing, then assume the state as low (0).
+- ti,vsel1-state-high: Inital state of vsel1 input is high.
+  If this property is missing, then assume the state as low (0).
+
+Any property defined as part of the core regulator binding, defined in
+regulator.txt, can also be used.
+
+Example:
+
+	abc: tps62360 {
+		compatible = "ti,tps62361";
+		reg =  <0x60>;
+		regulator-name = "tps62361-vout";
+		regulator-min-microvolt = <500000>;
+		regulator-max-microvolt = <1500000>;
+		regulator-boot-on
+		ti,vsel0-gpio = <&gpio1 16 0>;
+		ti,vsel1-gpio = <&gpio1 17 0>;
+		ti,vsel0-state-high;
+		ti,vsel1-state-high;
+		ti,enable-pull-down;
+		ti,enable-force-pwm;
+		ti,enable-vout-discharge;
+	};
diff --git a/drivers/regulator/tps62360-regulator.c b/drivers/regulator/tps62360-regulator.c
index 3506900..60765cc 100644
--- a/drivers/regulator/tps62360-regulator.c
+++ b/drivers/regulator/tps62360-regulator.c
@@ -26,6 +26,10 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/regulator/of_regulator.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
@@ -297,6 +301,56 @@ static const struct regmap_config tps62360_regmap_config = {
 	.cache_type		= REGCACHE_RBTREE,
 };
 
+static struct tps62360_regulator_platform_data *
+	of_get_tps62360_platform_data(struct device *dev)
+{
+	struct tps62360_regulator_platform_data *pdata;
+	struct device_node *np = dev->of_node;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(dev, "Memory alloc failed for platform data\n");
+		return NULL;
+	}
+
+	pdata->reg_init_data = of_get_regulator_init_data(dev, dev->of_node);
+	if (!pdata->reg_init_data) {
+		dev_err(dev, "Not able to get OF regulator init data\n");
+		return NULL;
+	}
+
+	pdata->vsel0_gpio = of_get_named_gpio(np, "vsel0-gpio", 0);
+	pdata->vsel1_gpio = of_get_named_gpio(np, "vsel1-gpio", 0);
+
+	if (of_find_property(np, "ti,vsel0-state-high", NULL))
+		pdata->vsel0_def_state = 1;
+
+	if (of_find_property(np, "ti,vsel1-state-high", NULL))
+		pdata->vsel1_def_state = 1;
+
+	if (of_find_property(np, "ti,enable-pull-down", NULL))
+		pdata->en_internal_pulldn = true;
+
+	if (of_find_property(np, "ti,enable-force-pwm", NULL))
+		pdata->en_force_pwm = true;
+
+	if (of_find_property(np, "ti,enable-vout-discharge", NULL))
+		pdata->en_discharge = true;
+
+	return pdata;
+}
+
+#if defined(CONFIG_OF)
+static const struct of_device_id tps62360_of_match[] = {
+	 { .compatible = "ti,tps62360", .data = (void *)TPS62360},
+	 { .compatible = "ti,tps62361", .data = (void *)TPS62361},
+	 { .compatible = "ti,tps62362", .data = (void *)TPS62362},
+	 { .compatible = "ti,tps62363", .data = (void *)TPS62363},
+	{},
+}
+MODULE_DEVICE_TABLE(of, tps62360_of_match);
+#endif
+
 static int __devinit tps62360_probe(struct i2c_client *client,
 				     const struct i2c_device_id *id)
 {
@@ -306,8 +360,24 @@ static int __devinit tps62360_probe(struct i2c_client *client,
 	struct tps62360_chip *tps;
 	int ret;
 	int i;
+	int chip_id;
 
 	pdata = client->dev.platform_data;
+	chip_id = id->driver_data;
+
+	if (client->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_device(of_match_ptr(tps62360_of_match),
+				&client->dev);
+		if (!match) {
+			dev_err(&client->dev, "Error: No device match found\n");
+			return -ENODEV;
+		}
+		chip_id = (int)match->data;
+		if (!pdata)
+			pdata = of_get_tps62360_platform_data(&client->dev);
+	}
+
 	if (!pdata) {
 		dev_err(&client->dev, "%s(): Platform data not found\n",
 						__func__);
@@ -328,7 +398,7 @@ static int __devinit tps62360_probe(struct i2c_client *client,
 	tps->vsel1_gpio = pdata->vsel1_gpio;
 	tps->dev = &client->dev;
 
-	switch (id->driver_data) {
+	switch (chip_id) {
 	case TPS62360:
 	case TPS62362:
 		tps->voltage_base = TPS62360_BASE_VOLTAGE;
@@ -484,6 +554,7 @@ static struct i2c_driver tps62360_i2c_driver = {
 	.driver = {
 		.name = "tps62360",
 		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(tps62360_of_match),
 	},
 	.probe = tps62360_probe,
 	.remove = __devexit_p(tps62360_remove),
-- 
1.7.1.1

Re: [PATCH V3 2/4] regulator: tps62360: add dt support

[Mark Brown]

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

On Fri, May 11, 2012 at 12:08:43PM +0530, Laxman Dewangan wrote:

This looks good overall but I do have a few things with the binding.

> +Optional properties:
> +- ti,enable-force-pwm: Enable force PWM mode. This is boolean value.

Hrm, this is fairly generic - it's REGULATOR_MODE_ACTIVE.  But I'm a bit
unsure about how generic exposing it is in bindings since it's often
dynamic in the running system, we've really not got any mainline
examples of something setting a mode.  With modern regulators the mode
detection stuff in the hardware is generally good enough that there's no
benefit from doing this, it's suprising to actually see systems that
benefit.

> +- ti,enable-vout-discharge: Enable output discharge. This is boolean value.

This I think we should definitely add a framework feature for this and
make into a generic property, it's a very standard feature and more
normally set unconditionally.

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

Re: [PATCH V3 2/4] regulator: tps62360: add dt support

[Laxman Dewangan]

On Friday 11 May 2012 08:28 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Fri, May 11, 2012 at 12:08:43PM +0530, Laxman Dewangan wrote:
>
> This looks good overall but I do have a few things with the binding.
>
>> +Optional properties:
>> +- ti,enable-force-pwm: Enable force PWM mode. This is boolean value.
> Hrm, this is fairly generic - it's REGULATOR_MODE_ACTIVE.  But I'm a bit
> unsure about how generic exposing it is in bindings since it's often
> dynamic in the running system, we've really not got any mainline
> examples of something setting a mode.  With modern regulators the mode
> detection stuff in the hardware is generally good enough that there's no
> benefit from doing this, it's suprising to actually see systems that
> benefit.
>

Yaah, I think this flag can map directly to REGULATOR_MODE_FAST. If I 
understand PWM mode properly then this is used when high current load is 
require or fast switching on  load current is require. By enabling force 
PWM enable, hw will not switch to PFM mode based on load current.

I think if we map the regulator mode to FAST as the force PWM enable and 
NORMAL as force PWM =0 then it will be generic.
Client can pass the initial mode when they fill constraint at the time 
of registration and it can also give the opportunity to change mode by 
calling regulator_set_mode().
If this is fine then I can create patch for removing this flag from 
platform data and add set_mode/get_mode function.

>> +- ti,enable-vout-discharge: Enable output discharge. This is boolean value.
> This I think we should definitely add a framework feature for this and
> make into a generic property, it's a very standard feature and more
> normally set unconditionally.
>

I think this is not require to enable always otherwise there may be 
power dissipation from this path always, just when we off the rail 
(disable rail or when go to shutdown so that the voltage output can go 
down faster).
Should we add "unsigned en_discharge:1" in regulator init data but did 
not get how core driver can use this flag specially when shutdown?

Re: [PATCH V3 2/4] regulator: tps62360: add dt support

[Laxman Dewangan]

On Friday 11 May 2012 08:28 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Fri, May 11, 2012 at 12:08:43PM +0530, Laxman Dewangan wrote:
>
> This looks good overall but I do have a few things with the binding.
>
>> +Optional properties:
>> +- ti,enable-force-pwm: Enable force PWM mode. This is boolean value.
> Hrm, this is fairly generic - it's REGULATOR_MODE_ACTIVE.  But I'm a bit
> unsure about how generic exposing it is in bindings since it's often
> dynamic in the running system, we've really not got any mainline
> examples of something setting a mode.  With modern regulators the mode
> detection stuff in the hardware is generally good enough that there's no
> benefit from doing this, it's suprising to actually see systems that
> benefit.
>

Yaah, I think this flag can map directly to REGULATOR_MODE_FAST. If I 
understand PWM mode properly then this is used when high current load is 
require or fast switching on  load current is require. By enabling force 
PWM enable, hw will not switch to PFM mode based on load current.

I think if we map the regulator mode to FAST as the force PWM enable and 
NORMAL as force PWM =0 then it will be generic.
Client can pass the initial mode when they fill constraint at the time 
of registration and it can also give the opportunity to change mode by 
calling regulator_set_mode().
If this is fine then I can create patch for removing this flag from 
platform data and add set_mode/get_mode function.

>> +- ti,enable-vout-discharge: Enable output discharge. This is boolean value.
> This I think we should definitely add a framework feature for this and
> make into a generic property, it's a very standard feature and more
> normally set unconditionally.
>

I think this is not require to enable always otherwise there may be 
power dissipation from this path always, just when we off the rail 
(disable rail or when go to shutdown so that the voltage output can go 
down faster).
Should we add "unsigned en_discharge:1" in regulator init data but did 
not get how core driver can use this flag specially when shutdown?

Re: [PATCH V3 2/4] regulator: tps62360: add dt support

[Mark Brown]

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

On Fri, May 11, 2012 at 09:05:01PM +0530, Laxman Dewangan wrote:

> Yaah, I think this flag can map directly to REGULATOR_MODE_FAST. If
> I understand PWM mode properly then this is used when high current
> load is require or fast switching on  load current is require. By
> enabling force PWM enable, hw will not switch to PFM mode based on
> load current.

> I think if we map the regulator mode to FAST as the force PWM enable
> and NORMAL as force PWM =0 then it will be generic.

Yes, this is pretty much exactly what _FAST means.  With modern hardware
the forced PWM mode just isn't needed, a modern regulator is able to
respond sufficiently quickly to changes in the output load.  Mode
switching from software was generally found to be impractical for DCDC
convertors as the load requirements change so quickly and without
software control that by the time you respond from software you can't
switch out of a lower power mode into a higher current one it's too late.

> >>+- ti,enable-vout-discharge: Enable output discharge. This is boolean value.
> >This I think we should definitely add a framework feature for this and
> >make into a generic property, it's a very standard feature and more
> >normally set unconditionally.

> I think this is not require to enable always otherwise there may be
> power dissipation from this path always, just when we off the rail
> (disable rail or when go to shutdown so that the voltage output can
> go down faster).
> Should we add "unsigned en_discharge:1" in regulator init data but
> did not get how core driver can use this flag specially when
> shutdown?

Oh, now you say this I think I remember discussing this with you before!
This is pretty unusual for such hardware.  Normally there's something in
the digital block so that the discharge is only enabled if the regulator
is disabled, if it's enabled then the discharge would automatically be
taken off since actively discharging while enabled doesn't make sense
and may in fact be actively harmful.

If the hardware doesn't do that then I'd expect the driver to take care
of this if it implements the framework feature (once the framework
feature is added).

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

Re: [PATCH V3 2/4] regulator: tps62360: add dt support

[Mark Brown]

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

On Fri, May 11, 2012 at 09:05:01PM +0530, Laxman Dewangan wrote:

> Yaah, I think this flag can map directly to REGULATOR_MODE_FAST. If
> I understand PWM mode properly then this is used when high current
> load is require or fast switching on  load current is require. By
> enabling force PWM enable, hw will not switch to PFM mode based on
> load current.

> I think if we map the regulator mode to FAST as the force PWM enable
> and NORMAL as force PWM =0 then it will be generic.

Yes, this is pretty much exactly what _FAST means.  With modern hardware
the forced PWM mode just isn't needed, a modern regulator is able to
respond sufficiently quickly to changes in the output load.  Mode
switching from software was generally found to be impractical for DCDC
convertors as the load requirements change so quickly and without
software control that by the time you respond from software you can't
switch out of a lower power mode into a higher current one it's too late.

> >>+- ti,enable-vout-discharge: Enable output discharge. This is boolean value.
> >This I think we should definitely add a framework feature for this and
> >make into a generic property, it's a very standard feature and more
> >normally set unconditionally.

> I think this is not require to enable always otherwise there may be
> power dissipation from this path always, just when we off the rail
> (disable rail or when go to shutdown so that the voltage output can
> go down faster).
> Should we add "unsigned en_discharge:1" in regulator init data but
> did not get how core driver can use this flag specially when
> shutdown?

Oh, now you say this I think I remember discussing this with you before!
This is pretty unusual for such hardware.  Normally there's something in
the digital block so that the discharge is only enabled if the regulator
is disabled, if it's enabled then the discharge would automatically be
taken off since actively discharging while enabled doesn't make sense
and may in fact be actively harmful.

If the hardware doesn't do that then I'd expect the driver to take care
of this if it implements the framework feature (once the framework
feature is added).

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

Re: [PATCH V3 2/4] regulator: tps62360: add dt support

[Laxman Dewangan]

On Saturday 12 May 2012 03:00 AM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Fri, May 11, 2012 at 09:05:01PM +0530, Laxman Dewangan wrote:
>
>> I think this is not require to enable always otherwise there may be
>> power dissipation from this path always, just when we off the rail
>> (disable rail or when go to shutdown so that the voltage output can
>> go down faster).
>> Should we add "unsigned en_discharge:1" in regulator init data but
>> did not get how core driver can use this flag specially when
>> shutdown?
> Oh, now you say this I think I remember discussing this with you before!
> This is pretty unusual for such hardware.  Normally there's something in
> the digital block so that the discharge is only enabled if the regulator
> is disabled, if it's enabled then the discharge would automatically be
> taken off since actively discharging while enabled doesn't make sense
> and may in fact be actively harmful.
>

Just to share  here with our new pmu which we got recently, the 
discharge path behavior is exactly same as what Mark mentioned
It need to be enable always. The pmu auto disable the DISC path in 
normal operation so there is no power dissipation but when it detects 
the en_bit is written 0 or any external control is making the rail to be 
off, it automatically turn on the path so that ramp-down can happen faster.
Just we need to enable this always.

> If the hardware doesn't do that then I'd expect the driver to take care
> of this if it implements the framework feature (once the framework
> feature is added).

Re: [PATCH V3 2/4] regulator: tps62360: add dt support

[Laxman Dewangan]

On Saturday 12 May 2012 03:00 AM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Fri, May 11, 2012 at 09:05:01PM +0530, Laxman Dewangan wrote:
>
>> I think this is not require to enable always otherwise there may be
>> power dissipation from this path always, just when we off the rail
>> (disable rail or when go to shutdown so that the voltage output can
>> go down faster).
>> Should we add "unsigned en_discharge:1" in regulator init data but
>> did not get how core driver can use this flag specially when
>> shutdown?
> Oh, now you say this I think I remember discussing this with you before!
> This is pretty unusual for such hardware.  Normally there's something in
> the digital block so that the discharge is only enabled if the regulator
> is disabled, if it's enabled then the discharge would automatically be
> taken off since actively discharging while enabled doesn't make sense
> and may in fact be actively harmful.
>

Just to share  here with our new pmu which we got recently, the 
discharge path behavior is exactly same as what Mark mentioned
It need to be enable always. The pmu auto disable the DISC path in 
normal operation so there is no power dissipation but when it detects 
the en_bit is written 0 or any external control is making the rail to be 
off, it automatically turn on the path so that ramp-down can happen faster.
Just we need to enable this always.

> If the hardware doesn't do that then I'd expect the driver to take care
> of this if it implements the framework feature (once the framework
> feature is added).

Re: [PATCH V3 2/4] regulator: tps62360: add dt support

[Mark Brown]

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

On Fri, May 11, 2012 at 12:08:43PM +0530, Laxman Dewangan wrote:
> Add dt support for the pmu device tps62360 and
> Add binding documentation with example.
> With this patch driver will support both device-tree and
> non-device tree registration.

Oh, sorry - I meant to say that I went ahead and applied this.  Ideally
the active discharge thing will get factored out quickly though.

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

[PATCH V3 3/4] ARM: tegra: config: enable REGULATOR_TPS62360

[Laxman Dewangan]

Enable regulator REGULATOR_TPS62360 for the core power
of cardhu.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
No changes from V1 and V2.

 arch/arm/configs/tegra_defconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index 7a90abb..415ada1 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -114,6 +114,7 @@ CONFIG_REGULATOR=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
 CONFIG_REGULATOR_VIRTUAL_CONSUMER=y
 CONFIG_REGULATOR_GPIO=y
+CONFIG_REGULATOR_TPS62360=y
 CONFIG_REGULATOR_TPS6586X=y
 CONFIG_SOUND=y
 CONFIG_SND=y
-- 
1.7.1.1

[PATCH V3 3/4] ARM: tegra: config: enable REGULATOR_TPS62360

[Laxman Dewangan]

Enable regulator REGULATOR_TPS62360 for the core power
of cardhu.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
No changes from V1 and V2.

 arch/arm/configs/tegra_defconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index 7a90abb..415ada1 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -114,6 +114,7 @@ CONFIG_REGULATOR=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
 CONFIG_REGULATOR_VIRTUAL_CONSUMER=y
 CONFIG_REGULATOR_GPIO=y
+CONFIG_REGULATOR_TPS62360=y
 CONFIG_REGULATOR_TPS6586X=y
 CONFIG_SOUND=y
 CONFIG_SND=y
-- 
1.7.1.1

[PATCH V3 4/4] ARM: dt: tegra: cardhu: register core regulator tps62360

[Laxman Dewangan]

Add device info for the core regulator tps62360 in tegra-cardhu
dts file.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
No changes from V1 and V2.

 arch/arm/boot/dts/tegra-cardhu.dts |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/tegra-cardhu.dts b/arch/arm/boot/dts/tegra-cardhu.dts
index facb950..19b6e93 100644
--- a/arch/arm/boot/dts/tegra-cardhu.dts
+++ b/arch/arm/boot/dts/tegra-cardhu.dts
@@ -123,6 +123,20 @@
 			micdet-delay = <100>;
 			gpio-cfg = <0xffffffff 0xffffffff 0 0xffffffff 0xffffffff>;
 		};
+
+		tps62360 {
+			compatible = "ti,tps62361";
+			reg = <0x60>;
+
+			regulator-name = "tps62361-vout";
+			regulator-min-microvolt = <500000>;
+			regulator-max-microvolt = <1500000>;
+			regulator-boot-on;
+			regulator-always-on;
+			ti,vsel0-state-high;
+			ti,vsel1-state-high;
+		};
+
 	};
 
 	sdhci@78000000 {
-- 
1.7.1.1

[PATCH V3 4/4] ARM: dt: tegra: cardhu: register core regulator tps62360

[Laxman Dewangan]

Add device info for the core regulator tps62360 in tegra-cardhu
dts file.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
No changes from V1 and V2.

 arch/arm/boot/dts/tegra-cardhu.dts |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/tegra-cardhu.dts b/arch/arm/boot/dts/tegra-cardhu.dts
index facb950..19b6e93 100644
--- a/arch/arm/boot/dts/tegra-cardhu.dts
+++ b/arch/arm/boot/dts/tegra-cardhu.dts
@@ -123,6 +123,20 @@
 			micdet-delay = <100>;
 			gpio-cfg = <0xffffffff 0xffffffff 0 0xffffffff 0xffffffff>;
 		};
+
+		tps62360 {
+			compatible = "ti,tps62361";
+			reg = <0x60>;
+
+			regulator-name = "tps62361-vout";
+			regulator-min-microvolt = <500000>;
+			regulator-max-microvolt = <1500000>;
+			regulator-boot-on;
+			regulator-always-on;
+			ti,vsel0-state-high;
+			ti,vsel1-state-high;
+		};
+
 	};
 
 	sdhci@78000000 {
-- 
1.7.1.1