[PATCH V2 0/4] regulator: tps65090: fix regulator registration and add external control support

[PATCH V2 0/4] regulator: tps65090: fix regulator registration and add external control support

[Laxman Dewangan]

This patch series fixes the regulator registration issue and add support
for external control.
Patch 1: It renames the driver name and regulators to more appropriate.
Patch 2: Fix the issue with regulator registration.
Patch 3: Add LDO support.
Patch 4: Add voltage output level information.
Patch 5: Add support for extenal control for DCDC.

Changes from V1:
- Shuffle patch 1 and 2 to make patch2 to patch1.
- Remove patch 4 for output voltage level.
- Move some of implementation from patch 5 to new patch1 for patch1
  completion.
- Move the content of tps65090-regulator.h to mfd/tps65090.h and remove
  this file.

Laxman Dewangan (4):
  regulator; tps65090: Register all regulators in single probe call
  regulator: tps65090: rename driver name and regulator name
  regulator: tps65090: Add support for LDO regulators
  regulator: tps65090: add external control support for DCDC

 drivers/regulator/tps65090-regulator.c       |  245 +++++++++++++++++++-------
 include/linux/mfd/tps65090.h                 |   35 ++++
 include/linux/regulator/tps65090-regulator.h |   50 ------
 3 files changed, 218 insertions(+), 112 deletions(-)
 delete mode 100644 include/linux/regulator/tps65090-regulator.h

[PATCH V2 1/4] regulator; tps65090: Register all regulators in single probe call

[Laxman Dewangan]

MFD driver registers the regulator driver once per device and
hence it is require to register all regulators in single probe
call.
Following are details of changes done to achieve this:
- Move the regulator enums to mfd header and remove the
  tps65090-regulator.h as it does not contain more info.
- Add max regulator and register all regulators even if there
  is no regulator init data from platform.
- Convert regulator init data to pointer type in platform data.
- Add input supply name in regulator desc to provide input supply.
- Separate desc information from driver information.
- Disable external control bit to have control through register write.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
Changes from V1: 
- Move this change to Patch1 from older patch2.
- Disable external control on init sequence.
- Move content of tps65090-regualtor.h to mfd/tps65090.h.

 drivers/regulator/tps65090-regulator.c       |  195 ++++++++++++++++++--------
 include/linux/mfd/tps65090.h                 |   28 ++++
 include/linux/regulator/tps65090-regulator.h |   50 -------
 3 files changed, 165 insertions(+), 108 deletions(-)
 delete mode 100644 include/linux/regulator/tps65090-regulator.h

diff --git a/drivers/regulator/tps65090-regulator.c b/drivers/regulator/tps65090-regulator.c
index 001ad55..5f7f931 100644
--- a/drivers/regulator/tps65090-regulator.c
+++ b/drivers/regulator/tps65090-regulator.c
@@ -24,15 +24,11 @@
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
 #include <linux/mfd/tps65090.h>
-#include <linux/regulator/tps65090-regulator.h>
 
 struct tps65090_regulator {
-	int		id;
-	/* used by regulator core */
-	struct regulator_desc	desc;
-
-	/* Device */
 	struct device		*dev;
+	struct regulator_desc	*desc;
+	struct regulator_dev	*rdev;
 };
 
 static struct regulator_ops tps65090_ops = {
@@ -41,44 +37,80 @@ static struct regulator_ops tps65090_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 };
 
-#define tps65090_REG(_id)				\
+#define tps65090_REG_DESC(_id, _sname, _en_reg, _ops)	\
 {							\
-	.id		= TPS65090_ID_##_id,		\
-	.desc = {					\
-		.name = tps65090_rails(_id),		\
-		.id = TPS65090_ID_##_id,		\
-		.ops = &tps65090_ops,			\
-		.type = REGULATOR_VOLTAGE,		\
-		.owner = THIS_MODULE,			\
-		.enable_reg = (TPS65090_ID_##_id) + 12,	\
-		.enable_mask = BIT(0),			\
-	},						\
+	.name = "TPS65090_RAILS"#_id,			\
+	.supply_name = _sname,				\
+	.id = TPS65090_ID_##_id,			\
+	.ops = &_ops,					\
+	.enable_reg = _en_reg,				\
+	.enable_mask = BIT(0),				\
+	.type = REGULATOR_VOLTAGE,			\
+	.owner = THIS_MODULE,				\
 }
 
-static struct tps65090_regulator TPS65090_regulator[] = {
-	tps65090_REG(DCDC1),
-	tps65090_REG(DCDC2),
-	tps65090_REG(DCDC3),
-	tps65090_REG(FET1),
-	tps65090_REG(FET2),
-	tps65090_REG(FET3),
-	tps65090_REG(FET4),
-	tps65090_REG(FET5),
-	tps65090_REG(FET6),
-	tps65090_REG(FET7),
+static struct regulator_desc tps65090_regulator_desc[] = {
+	tps65090_REG_DESC(DCDC1, "vsys1",   0x0C, tps65090_ops),
+	tps65090_REG_DESC(DCDC2, "vsys2",   0x0D, tps65090_ops),
+	tps65090_REG_DESC(DCDC3, "vsys3",   0x0E, tps65090_ops),
+	tps65090_REG_DESC(FET1,  "infet1",  0x0F, tps65090_ops),
+	tps65090_REG_DESC(FET2,  "infet2",  0x10, tps65090_ops),
+	tps65090_REG_DESC(FET3,  "infet3",  0x11, tps65090_ops),
+	tps65090_REG_DESC(FET4,  "infet4",  0x12, tps65090_ops),
+	tps65090_REG_DESC(FET5,  "infet5",  0x13, tps65090_ops),
+	tps65090_REG_DESC(FET6,  "infet6",  0x14, tps65090_ops),
+	tps65090_REG_DESC(FET7,  "infet7",  0x15, tps65090_ops),
 };
 
-static inline struct tps65090_regulator *find_regulator_info(int id)
+static inline bool is_dcdc(int id)
 {
-	struct tps65090_regulator *ri;
-	int i;
+	switch (id) {
+	case TPS65090_ID_DCDC1:
+	case TPS65090_ID_DCDC2:
+	case TPS65090_ID_DCDC3:
+		return true;
+	default:
+		return false;
+	}
+}
 
-	for (i = 0; i < ARRAY_SIZE(TPS65090_regulator); i++) {
-		ri = &TPS65090_regulator[i];
-		if (ri->desc.id == id)
-			return ri;
+static int __devinit tps65090_config_ext_control(
+	struct tps65090_regulator *ri, bool enable)
+{
+	int ret;
+	struct device *parent = ri->dev->parent;
+	unsigned int reg_en_reg = ri->desc->enable_reg;
+
+	if (enable)
+		ret = tps65090_set_bits(parent, reg_en_reg, 1);
+	else
+		ret =  tps65090_clr_bits(parent, reg_en_reg, 1);
+	if (ret < 0)
+		dev_err(ri->dev, "Error in updating reg 0x%x\n", reg_en_reg);
+	return ret;
+}
+
+static int __devinit tps65090_regulator_disable_ext_control(
+		struct tps65090_regulator *ri,
+		struct tps65090_regulator_plat_data *tps_pdata)
+{
+	int ret = 0;
+	struct device *parent = ri->dev->parent;
+	unsigned int reg_en_reg = ri->desc->enable_reg;
+
+	/*
+	 * First enable output for internal control if require.
+	 * And then disable external control.
+	 */
+	if (tps_pdata->reg_init_data->constraints.always_on ||
+			tps_pdata->reg_init_data->constraints.boot_on) {
+		ret =  tps65090_set_bits(parent, reg_en_reg, 0);
+		if (ret < 0) {
+			dev_err(ri->dev, "Error in set reg 0x%x\n", reg_en_reg);
+			return ret;
+		}
 	}
-	return NULL;
+	return tps65090_config_ext_control(ri, false);
 }
 
 static int __devinit tps65090_regulator_probe(struct platform_device *pdev)
@@ -87,40 +119,87 @@ static int __devinit tps65090_regulator_probe(struct platform_device *pdev)
 	struct tps65090_regulator *ri = NULL;
 	struct regulator_config config = { };
 	struct regulator_dev *rdev;
-	struct tps65090_regulator_platform_data *tps_pdata;
-	int id = pdev->id;
+	struct tps65090_regulator_plat_data *tps_pdata;
+	struct tps65090_regulator *pmic;
+	struct tps65090_platform_data *tps65090_pdata;
+	int num;
+	int ret;
 
-	dev_dbg(&pdev->dev, "Probing regulator %d\n", id);
+	dev_dbg(&pdev->dev, "Probing regulator\n");
 
-	ri = find_regulator_info(id);
-	if (ri == NULL) {
-		dev_err(&pdev->dev, "invalid regulator ID specified\n");
+	tps65090_pdata = dev_get_platdata(pdev->dev.parent);
+	if (!tps65090_pdata) {
+		dev_err(&pdev->dev, "Platform data missing\n");
 		return -EINVAL;
 	}
-	tps_pdata = pdev->dev.platform_data;
-	ri->dev = &pdev->dev;
-
-	config.dev = &pdev->dev;
-	config.init_data = &tps_pdata->regulator;
-	config.driver_data = ri;
-	config.regmap = tps65090_mfd->rmap;
-
-	rdev = regulator_register(&ri->desc, &config);
-	if (IS_ERR(rdev)) {
-		dev_err(&pdev->dev, "failed to register regulator %s\n",
-				ri->desc.name);
-		return PTR_ERR(rdev);
+
+	pmic = devm_kzalloc(&pdev->dev, TPS65090_ID_MAX * sizeof(*pmic),
+			GFP_KERNEL);
+	if (!pmic) {
+		dev_err(&pdev->dev, "mem alloc for pmic failed\n");
+		return -ENOMEM;
+	}
+
+	for (num = 0; num < TPS65090_ID_MAX; num++) {
+		tps_pdata = tps65090_pdata->reg_pdata[num];
+
+		ri = &pmic[num];
+		ri->dev = &pdev->dev;
+		ri->desc = &tps65090_regulator_desc[num];
+
+		/*
+		 * TPS5090 DCDC support the control from external digital input.
+		 * It may be possible that during boot, the external control is
+		 * enabled. Disabling external control for DCDC.
+		 */
+		if (tps_pdata && is_dcdc(num) && tps_pdata->reg_init_data) {
+			ret = tps65090_regulator_disable_ext_control(
+						ri, tps_pdata);
+			if (ret < 0) {
+				dev_err(&pdev->dev,
+						"failed disable ext control\n");
+				goto scrub;
+			}
+		}
+		config.dev = &pdev->dev;
+		config.driver_data = ri;
+		config.regmap = tps65090_mfd->rmap;
+		if (tps_pdata)
+			config.init_data = tps_pdata->reg_init_data;
+		else
+			config.init_data = NULL;
+
+		rdev = regulator_register(ri->desc, &config);
+		if (IS_ERR(rdev)) {
+			dev_err(&pdev->dev, "failed to register regulator %s\n",
+				ri->desc->name);
+			ret = PTR_ERR(rdev);
+			goto scrub;
+		}
+		ri->rdev = rdev;
 	}
 
-	platform_set_drvdata(pdev, rdev);
+	platform_set_drvdata(pdev, pmic);
 	return 0;
+
+scrub:
+	while (--num >= 0) {
+		ri = &pmic[num];
+		regulator_unregister(ri->rdev);
+	}
+	return ret;
 }
 
 static int __devexit tps65090_regulator_remove(struct platform_device *pdev)
 {
-	struct regulator_dev *rdev = platform_get_drvdata(pdev);
+	struct tps65090_regulator *pmic = platform_get_drvdata(pdev);
+	struct tps65090_regulator *ri;
+	int num;
 
-	regulator_unregister(rdev);
+	for (num = 0; num < TPS65090_ID_MAX; ++num) {
+		ri = &pmic[num];
+		regulator_unregister(ri->rdev);
+	}
 	return 0;
 }
 
diff --git a/include/linux/mfd/tps65090.h b/include/linux/mfd/tps65090.h
index 6bc31d8..d06c633 100644
--- a/include/linux/mfd/tps65090.h
+++ b/include/linux/mfd/tps65090.h
@@ -24,6 +24,23 @@
 
 #include <linux/irq.h>
 
+/* TPS65090 Regulator ID */
+enum {
+	TPS65090_ID_DCDC1,
+	TPS65090_ID_DCDC2,
+	TPS65090_ID_DCDC3,
+	TPS65090_ID_FET1,
+	TPS65090_ID_FET2,
+	TPS65090_ID_FET3,
+	TPS65090_ID_FET4,
+	TPS65090_ID_FET5,
+	TPS65090_ID_FET6,
+	TPS65090_ID_FET7,
+
+	/* Last entry for maximum ID */
+	TPS65090_ID_MAX,
+};
+
 struct tps65090 {
 	struct mutex		lock;
 	struct device		*dev;
@@ -41,10 +58,21 @@ struct tps65090_subdev_info {
 	void		*platform_data;
 };
 
+/*
+ * struct tps65090_regulator_plat_data
+ *
+ * @reg_init_data: The regulator init data.
+ */
+
+struct tps65090_regulator_plat_data {
+	struct regulator_init_data *reg_init_data;
+};
+
 struct tps65090_platform_data {
 	int irq_base;
 	int num_subdevs;
 	struct tps65090_subdev_info *subdevs;
+	struct tps65090_regulator_plat_data *reg_pdata[TPS65090_ID_MAX];
 };
 
 /*
diff --git a/include/linux/regulator/tps65090-regulator.h b/include/linux/regulator/tps65090-regulator.h
deleted file mode 100644
index 0fa04b6..0000000
--- a/include/linux/regulator/tps65090-regulator.h
+++ /dev/null
@@ -1,50 +0,0 @@
-/*
- * Regulator driver interface for TI TPS65090 PMIC family
- *
- * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
-
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
-
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
-
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef __REGULATOR_TPS65090_H
-#define __REGULATOR_TPS65090_H
-
-#include <linux/regulator/machine.h>
-
-#define tps65090_rails(_name) "tps65090_"#_name
-
-enum {
-	TPS65090_ID_DCDC1,
-	TPS65090_ID_DCDC2,
-	TPS65090_ID_DCDC3,
-	TPS65090_ID_FET1,
-	TPS65090_ID_FET2,
-	TPS65090_ID_FET3,
-	TPS65090_ID_FET4,
-	TPS65090_ID_FET5,
-	TPS65090_ID_FET6,
-	TPS65090_ID_FET7,
-};
-
-/*
- * struct tps65090_regulator_platform_data
- *
- * @regulator: The regulator init data.
- * @slew_rate_uV_per_us: Slew rate microvolt per microsec.
- */
-
-struct tps65090_regulator_platform_data {
-	struct regulator_init_data regulator;
-};
-
-#endif	/* __REGULATOR_TPS65090_H */
-- 
1.7.1.1

[PATCH V2 2/4] regulator: tps65090: rename driver name and regulator name

[Laxman Dewangan]

To make the names proper and more appropriate:
Rename the driver name from tps65090-regulator to tps65090-pmic.
Rename the regulators from TPS65090_ID_* to TPS65090_REGULATOR_*

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
Changes from V1: 
Make this as patch2 from older patch1.

 drivers/regulator/tps65090-regulator.c |   16 ++++++++--------
 include/linux/mfd/tps65090.h           |   24 ++++++++++++------------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/regulator/tps65090-regulator.c b/drivers/regulator/tps65090-regulator.c
index 5f7f931..584a185 100644
--- a/drivers/regulator/tps65090-regulator.c
+++ b/drivers/regulator/tps65090-regulator.c
@@ -41,7 +41,7 @@ static struct regulator_ops tps65090_ops = {
 {							\
 	.name = "TPS65090_RAILS"#_id,			\
 	.supply_name = _sname,				\
-	.id = TPS65090_ID_##_id,			\
+	.id = TPS65090_REGULATOR_##_id,			\
 	.ops = &_ops,					\
 	.enable_reg = _en_reg,				\
 	.enable_mask = BIT(0),				\
@@ -65,9 +65,9 @@ static struct regulator_desc tps65090_regulator_desc[] = {
 static inline bool is_dcdc(int id)
 {
 	switch (id) {
-	case TPS65090_ID_DCDC1:
-	case TPS65090_ID_DCDC2:
-	case TPS65090_ID_DCDC3:
+	case TPS65090_REGULATOR_DCDC1:
+	case TPS65090_REGULATOR_DCDC2:
+	case TPS65090_REGULATOR_DCDC3:
 		return true;
 	default:
 		return false;
@@ -133,14 +133,14 @@ static int __devinit tps65090_regulator_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	pmic = devm_kzalloc(&pdev->dev, TPS65090_ID_MAX * sizeof(*pmic),
+	pmic = devm_kzalloc(&pdev->dev, TPS65090_REGULATOR_MAX * sizeof(*pmic),
 			GFP_KERNEL);
 	if (!pmic) {
 		dev_err(&pdev->dev, "mem alloc for pmic failed\n");
 		return -ENOMEM;
 	}
 
-	for (num = 0; num < TPS65090_ID_MAX; num++) {
+	for (num = 0; num < TPS65090_REGULATOR_MAX; num++) {
 		tps_pdata = tps65090_pdata->reg_pdata[num];
 
 		ri = &pmic[num];
@@ -196,7 +196,7 @@ static int __devexit tps65090_regulator_remove(struct platform_device *pdev)
 	struct tps65090_regulator *ri;
 	int num;
 
-	for (num = 0; num < TPS65090_ID_MAX; ++num) {
+	for (num = 0; num < TPS65090_REGULATOR_MAX; ++num) {
 		ri = &pmic[num];
 		regulator_unregister(ri->rdev);
 	}
@@ -205,7 +205,7 @@ static int __devexit tps65090_regulator_remove(struct platform_device *pdev)
 
 static struct platform_driver tps65090_regulator_driver = {
 	.driver	= {
-		.name	= "tps65090-regulator",
+		.name	= "tps65090-pmic",
 		.owner	= THIS_MODULE,
 	},
 	.probe		= tps65090_regulator_probe,
diff --git a/include/linux/mfd/tps65090.h b/include/linux/mfd/tps65090.h
index d06c633..958bf15 100644
--- a/include/linux/mfd/tps65090.h
+++ b/include/linux/mfd/tps65090.h
@@ -26,19 +26,19 @@
 
 /* TPS65090 Regulator ID */
 enum {
-	TPS65090_ID_DCDC1,
-	TPS65090_ID_DCDC2,
-	TPS65090_ID_DCDC3,
-	TPS65090_ID_FET1,
-	TPS65090_ID_FET2,
-	TPS65090_ID_FET3,
-	TPS65090_ID_FET4,
-	TPS65090_ID_FET5,
-	TPS65090_ID_FET6,
-	TPS65090_ID_FET7,
+	TPS65090_REGULATOR_DCDC1,
+	TPS65090_REGULATOR_DCDC2,
+	TPS65090_REGULATOR_DCDC3,
+	TPS65090_REGULATOR_FET1,
+	TPS65090_REGULATOR_FET2,
+	TPS65090_REGULATOR_FET3,
+	TPS65090_REGULATOR_FET4,
+	TPS65090_REGULATOR_FET5,
+	TPS65090_REGULATOR_FET6,
+	TPS65090_REGULATOR_FET7,
 
 	/* Last entry for maximum ID */
-	TPS65090_ID_MAX,
+	TPS65090_REGULATOR_MAX,
 };
 
 struct tps65090 {
@@ -72,7 +72,7 @@ struct tps65090_platform_data {
 	int irq_base;
 	int num_subdevs;
 	struct tps65090_subdev_info *subdevs;
-	struct tps65090_regulator_plat_data *reg_pdata[TPS65090_ID_MAX];
+	struct tps65090_regulator_plat_data *reg_pdata[TPS65090_REGULATOR_MAX];
 };
 
 /*
-- 
1.7.1.1

[PATCH V2 3/4] regulator: tps65090: Add support for LDO regulators

[Laxman Dewangan]

TPS65090 supports the two LDOs, LDO1 and LDO2. These are
always ON regulators. The output on these LDOs are available
once the input voltage available for these LDOs.
Add support for these LDOs regulators.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
Changes from V1:
No change from older patch.

 drivers/regulator/tps65090-regulator.c |    5 +++++
 include/linux/mfd/tps65090.h           |    2 ++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/tps65090-regulator.c b/drivers/regulator/tps65090-regulator.c
index 584a185..0732d9b 100644
--- a/drivers/regulator/tps65090-regulator.c
+++ b/drivers/regulator/tps65090-regulator.c
@@ -37,6 +37,9 @@ static struct regulator_ops tps65090_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 };
 
+static struct regulator_ops tps65090_ldo_ops = {
+};
+
 #define tps65090_REG_DESC(_id, _sname, _en_reg, _ops)	\
 {							\
 	.name = "TPS65090_RAILS"#_id,			\
@@ -60,6 +63,8 @@ static struct regulator_desc tps65090_regulator_desc[] = {
 	tps65090_REG_DESC(FET5,  "infet5",  0x13, tps65090_ops),
 	tps65090_REG_DESC(FET6,  "infet6",  0x14, tps65090_ops),
 	tps65090_REG_DESC(FET7,  "infet7",  0x15, tps65090_ops),
+	tps65090_REG_DESC(LDO1,  "vsys_l1", 0,    tps65090_ldo_ops),
+	tps65090_REG_DESC(LDO2,  "vsys_l2", 0,    tps65090_ldo_ops),
 };
 
 static inline bool is_dcdc(int id)
diff --git a/include/linux/mfd/tps65090.h b/include/linux/mfd/tps65090.h
index 958bf15..5989212 100644
--- a/include/linux/mfd/tps65090.h
+++ b/include/linux/mfd/tps65090.h
@@ -36,6 +36,8 @@ enum {
 	TPS65090_REGULATOR_FET5,
 	TPS65090_REGULATOR_FET6,
 	TPS65090_REGULATOR_FET7,
+	TPS65090_REGULATOR_LDO1,
+	TPS65090_REGULATOR_LDO2,
 
 	/* Last entry for maximum ID */
 	TPS65090_REGULATOR_MAX,
-- 
1.7.1.1

[PATCH V2 4/4] regulator: tps65090: add external control support for DCDC

[Laxman Dewangan]

The TPS65090's DCDC output can also be enable/disable through the
external digital input signal. Add support for enable/disable
either through register access via I2C or through external
control inputs. The external control inputs can be driven through
GPIOs also and hence adding support for passing the GPIO number.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
Changes from V1:
Simplify external control implemetation.
 
 drivers/regulator/tps65090-regulator.c |   77 +++++++++++++++++++++++--------
 include/linux/mfd/tps65090.h           |    7 +++-
 2 files changed, 63 insertions(+), 21 deletions(-)

diff --git a/drivers/regulator/tps65090-regulator.c b/drivers/regulator/tps65090-regulator.c
index 0732d9b..4124138 100644
--- a/drivers/regulator/tps65090-regulator.c
+++ b/drivers/regulator/tps65090-regulator.c
@@ -18,6 +18,7 @@
 
 #include <linux/module.h>
 #include <linux/init.h>
+#include <linux/gpio.h>
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/platform_device.h>
@@ -31,10 +32,13 @@ struct tps65090_regulator {
 	struct regulator_dev	*rdev;
 };
 
-static struct regulator_ops tps65090_ops = {
-	.enable = regulator_enable_regmap,
-	.disable = regulator_disable_regmap,
-	.is_enabled = regulator_is_enabled_regmap,
+static struct regulator_ops tps65090_ext_control_ops = {
+};
+
+static struct regulator_ops tps65090_reg_contol_ops = {
+	.enable		= regulator_enable_regmap,
+	.disable	= regulator_disable_regmap,
+	.is_enabled	= regulator_is_enabled_regmap,
 };
 
 static struct regulator_ops tps65090_ldo_ops = {
@@ -53,16 +57,16 @@ static struct regulator_ops tps65090_ldo_ops = {
 }
 
 static struct regulator_desc tps65090_regulator_desc[] = {
-	tps65090_REG_DESC(DCDC1, "vsys1",   0x0C, tps65090_ops),
-	tps65090_REG_DESC(DCDC2, "vsys2",   0x0D, tps65090_ops),
-	tps65090_REG_DESC(DCDC3, "vsys3",   0x0E, tps65090_ops),
-	tps65090_REG_DESC(FET1,  "infet1",  0x0F, tps65090_ops),
-	tps65090_REG_DESC(FET2,  "infet2",  0x10, tps65090_ops),
-	tps65090_REG_DESC(FET3,  "infet3",  0x11, tps65090_ops),
-	tps65090_REG_DESC(FET4,  "infet4",  0x12, tps65090_ops),
-	tps65090_REG_DESC(FET5,  "infet5",  0x13, tps65090_ops),
-	tps65090_REG_DESC(FET6,  "infet6",  0x14, tps65090_ops),
-	tps65090_REG_DESC(FET7,  "infet7",  0x15, tps65090_ops),
+	tps65090_REG_DESC(DCDC1, "vsys1",   0x0C, tps65090_reg_contol_ops),
+	tps65090_REG_DESC(DCDC2, "vsys2",   0x0D, tps65090_reg_contol_ops),
+	tps65090_REG_DESC(DCDC3, "vsys3",   0x0E, tps65090_reg_contol_ops),
+	tps65090_REG_DESC(FET1,  "infet1",  0x0F, tps65090_reg_contol_ops),
+	tps65090_REG_DESC(FET2,  "infet2",  0x10, tps65090_reg_contol_ops),
+	tps65090_REG_DESC(FET3,  "infet3",  0x11, tps65090_reg_contol_ops),
+	tps65090_REG_DESC(FET4,  "infet4",  0x12, tps65090_reg_contol_ops),
+	tps65090_REG_DESC(FET5,  "infet5",  0x13, tps65090_reg_contol_ops),
+	tps65090_REG_DESC(FET6,  "infet6",  0x14, tps65090_reg_contol_ops),
+	tps65090_REG_DESC(FET7,  "infet7",  0x15, tps65090_reg_contol_ops),
 	tps65090_REG_DESC(LDO1,  "vsys_l1", 0,    tps65090_ldo_ops),
 	tps65090_REG_DESC(LDO2,  "vsys_l2", 0,    tps65090_ldo_ops),
 };
@@ -118,6 +122,22 @@ static int __devinit tps65090_regulator_disable_ext_control(
 	return tps65090_config_ext_control(ri, false);
 }
 
+static void __devinit tps65090_configure_regulator_config(
+		struct tps65090_regulator_plat_data *tps_pdata,
+		struct regulator_config *config)
+{
+	if (gpio_is_valid(tps_pdata->gpio)) {
+		int gpio_flag = GPIOF_OUT_INIT_LOW;
+
+		if (tps_pdata->reg_init_data->constraints.always_on ||
+				tps_pdata->reg_init_data->constraints.boot_on)
+			gpio_flag = GPIOF_OUT_INIT_HIGH;
+
+		config->ena_gpio = tps_pdata->gpio;
+		config->ena_gpio_flags = gpio_flag;
+	}
+}
+
 static int __devinit tps65090_regulator_probe(struct platform_device *pdev)
 {
 	struct tps65090 *tps65090_mfd = dev_get_drvdata(pdev->dev.parent);
@@ -154,18 +174,24 @@ static int __devinit tps65090_regulator_probe(struct platform_device *pdev)
 
 		/*
 		 * TPS5090 DCDC support the control from external digital input.
-		 * It may be possible that during boot, the external control is
-		 * enabled. Disabling external control for DCDC.
+		 * Configure it as per platform data.
 		 */
 		if (tps_pdata && is_dcdc(num) && tps_pdata->reg_init_data) {
-			ret = tps65090_regulator_disable_ext_control(
+			if (tps_pdata->enable_ext_control) {
+				tps65090_configure_regulator_config(
+						tps_pdata, &config);
+				ri->desc->ops = &tps65090_ext_control_ops;
+			} else {
+				ret = tps65090_regulator_disable_ext_control(
 						ri, tps_pdata);
-			if (ret < 0) {
-				dev_err(&pdev->dev,
+				if (ret < 0) {
+					dev_err(&pdev->dev,
 						"failed disable ext control\n");
-				goto scrub;
+					goto scrub;
+				}
 			}
 		}
+
 		config.dev = &pdev->dev;
 		config.driver_data = ri;
 		config.regmap = tps65090_mfd->rmap;
@@ -182,6 +208,17 @@ static int __devinit tps65090_regulator_probe(struct platform_device *pdev)
 			goto scrub;
 		}
 		ri->rdev = rdev;
+
+		/* Enable external control if it is require */
+		if (tps_pdata && is_dcdc(num) && tps_pdata->reg_init_data &&
+				tps_pdata->enable_ext_control) {
+			ret = tps65090_config_ext_control(ri, true);
+			if (ret < 0) {
+				/* Increment num to get unregister rdev */
+				num++;
+				goto scrub;
+			}
+		}
 	}
 
 	platform_set_drvdata(pdev, pmic);
diff --git a/include/linux/mfd/tps65090.h b/include/linux/mfd/tps65090.h
index 5989212..804e280 100644
--- a/include/linux/mfd/tps65090.h
+++ b/include/linux/mfd/tps65090.h
@@ -64,10 +64,15 @@ struct tps65090_subdev_info {
  * struct tps65090_regulator_plat_data
  *
  * @reg_init_data: The regulator init data.
+ * @enable_ext_control: Enable extrenal control or not. Only available for
+ *     DCDC1, DCDC2 and DCDC3.
+ * @gpio: Gpio number if external control is enabled and controlled through
+ *     gpio.
  */
-
 struct tps65090_regulator_plat_data {
 	struct regulator_init_data *reg_init_data;
+	bool enable_ext_control;
+	int gpio;
 };
 
 struct tps65090_platform_data {
-- 
1.7.1.1

Re: [PATCH V2 4/4] regulator: tps65090: add external control support for DCDC

[Laxman Dewangan]

On Tuesday 09 October 2012 04:03 PM, Venu Byravarasu wrote:
>> +static struct regulator_ops tps65090_ext_control_ops = {
>> +};
> What is the purpose of adding empty structure?
>
We will not able to register regulator if there is no ops.
In regulator register:
         if (regulator_desc->name == NULL || regulator_desc->ops == NULL)
                 return ERR_PTR(-EINVAL);

RE: [PATCH V2 4/4] regulator: tps65090: add external control support for DCDC

[Venu Byravarasu]

> -----Original Message-----
> From: Laxman Dewangan
> Sent: Tuesday, October 09, 2012 3:19 PM
> To: broonie@opensource.wolfsonmicro.com; lrg@ti.com
> Cc: sameo@linux.intel.com; Venu Byravarasu; linux-kernel@vger.kernel.org;
> Laxman Dewangan
> Subject: [PATCH V2 4/4] regulator: tps65090: add external control support for
> DCDC
> 
> The TPS65090's DCDC output can also be enable/disable through the
> external digital input signal. Add support for enable/disable
> either through register access via I2C or through external
> control inputs. The external control inputs can be driven through
> GPIOs also and hence adding support for passing the GPIO number.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
> Changes from V1:
> Simplify external control implemetation.
> 
> -static struct regulator_ops tps65090_ops = {
> -	.enable = regulator_enable_regmap,
> -	.disable = regulator_disable_regmap,
> -	.is_enabled = regulator_is_enabled_regmap,
> +static struct regulator_ops tps65090_ext_control_ops = {
> +};

What is the purpose of adding empty structure?

> +
> +static struct regulator_ops tps65090_reg_contol_ops = {
> +	.enable		= regulator_enable_regmap,
> +	.disable	= regulator_disable_regmap,
> +	.is_enabled	= regulator_is_enabled_regmap,
>  };
> 
>  static struct regulator_ops tps65090_ldo_ops = {
> @@ -53,16 +57,16 @@ static struct regulator_ops tps65090_ldo_ops = {
>  }
> 
>  static struct regulator_desc tps65090_regulator_desc[] = {
> -	tps65090_REG_DESC(DCDC1, "vsys1",   0x0C, tps65090_ops),
> -	tps65090_REG_DESC(DCDC2, "vsys2",   0x0D, tps65090_ops),
> -	tps65090_REG_DESC(DCDC3, "vsys3",   0x0E, tps65090_ops),
> -	tps65090_REG_DESC(FET1,  "infet1",  0x0F, tps65090_ops),
> -	tps65090_REG_DESC(FET2,  "infet2",  0x10, tps65090_ops),
> -	tps65090_REG_DESC(FET3,  "infet3",  0x11, tps65090_ops),
> -	tps65090_REG_DESC(FET4,  "infet4",  0x12, tps65090_ops),
> -	tps65090_REG_DESC(FET5,  "infet5",  0x13, tps65090_ops),
> -	tps65090_REG_DESC(FET6,  "infet6",  0x14, tps65090_ops),
> -	tps65090_REG_DESC(FET7,  "infet7",  0x15, tps65090_ops),
> +	tps65090_REG_DESC(DCDC1, "vsys1",   0x0C,
> tps65090_reg_contol_ops),
> +	tps65090_REG_DESC(DCDC2, "vsys2",   0x0D,
> tps65090_reg_contol_ops),
> +	tps65090_REG_DESC(DCDC3, "vsys3",   0x0E,
> tps65090_reg_contol_ops),
> +	tps65090_REG_DESC(FET1,  "infet1",  0x0F,
> tps65090_reg_contol_ops),
> +	tps65090_REG_DESC(FET2,  "infet2",  0x10,
> tps65090_reg_contol_ops),
> +	tps65090_REG_DESC(FET3,  "infet3",  0x11,
> tps65090_reg_contol_ops),
> +	tps65090_REG_DESC(FET4,  "infet4",  0x12,
> tps65090_reg_contol_ops),
> +	tps65090_REG_DESC(FET5,  "infet5",  0x13,
> tps65090_reg_contol_ops),
> +	tps65090_REG_DESC(FET6,  "infet6",  0x14,
> tps65090_reg_contol_ops),
> +	tps65090_REG_DESC(FET7,  "infet7",  0x15,
> tps65090_reg_contol_ops),
>  	tps65090_REG_DESC(LDO1,  "vsys_l1", 0,    tps65090_ldo_ops),
>  	tps65090_REG_DESC(LDO2,  "vsys_l2", 0,    tps65090_ldo_ops),
>  };
> @@ -118,6 +122,22 @@ static int __devinit
> tps65090_regulator_disable_ext_control(
>  	return tps65090_config_ext_control(ri, false);
>  }
> 
> +static void __devinit tps65090_configure_regulator_config(
> +		struct tps65090_regulator_plat_data *tps_pdata,
> +		struct regulator_config *config)
> +{
> +	if (gpio_is_valid(tps_pdata->gpio)) {
> +		int gpio_flag = GPIOF_OUT_INIT_LOW;
> +
> +		if (tps_pdata->reg_init_data->constraints.always_on ||
> +				tps_pdata->reg_init_data-
> >constraints.boot_on)
> +			gpio_flag = GPIOF_OUT_INIT_HIGH;
> +
> +		config->ena_gpio = tps_pdata->gpio;
> +		config->ena_gpio_flags = gpio_flag;
> +	}
> +}
> +
>  static int __devinit tps65090_regulator_probe(struct platform_device *pdev)
>  {
>  	struct tps65090 *tps65090_mfd = dev_get_drvdata(pdev-
> >dev.parent);
> @@ -154,18 +174,24 @@ static int __devinit
> tps65090_regulator_probe(struct platform_device *pdev)
> 
>  		/*
>  		 * TPS5090 DCDC support the control from external digital
> input.
> -		 * It may be possible that during boot, the external control is
> -		 * enabled. Disabling external control for DCDC.
> +		 * Configure it as per platform data.
>  		 */
>  		if (tps_pdata && is_dcdc(num) && tps_pdata->reg_init_data)
> {
> -			ret = tps65090_regulator_disable_ext_control(
> +			if (tps_pdata->enable_ext_control) {
> +				tps65090_configure_regulator_config(
> +						tps_pdata, &config);
> +				ri->desc->ops = &tps65090_ext_control_ops;
> +			} else {
> +				ret =
> tps65090_regulator_disable_ext_control(
>  						ri, tps_pdata);
> -			if (ret < 0) {
> -				dev_err(&pdev->dev,
> +				if (ret < 0) {
> +					dev_err(&pdev->dev,
>  						"failed disable ext
> control\n");
> -				goto scrub;
> +					goto scrub;
> +				}
>  			}
>  		}
> +
>  		config.dev = &pdev->dev;
>  		config.driver_data = ri;
>  		config.regmap = tps65090_mfd->rmap;
> @@ -182,6 +208,17 @@ static int __devinit tps65090_regulator_probe(struct
> platform_device *pdev)
>  			goto scrub;
>  		}
>  		ri->rdev = rdev;
> +
> +		/* Enable external control if it is require */
> +		if (tps_pdata && is_dcdc(num) && tps_pdata->reg_init_data
> &&
> +				tps_pdata->enable_ext_control) {
> +			ret = tps65090_config_ext_control(ri, true);
> +			if (ret < 0) {
> +				/* Increment num to get unregister rdev */
> +				num++;
> +				goto scrub;
> +			}
> +		}
>  	}
> 
>  	platform_set_drvdata(pdev, pmic);
> diff --git a/include/linux/mfd/tps65090.h b/include/linux/mfd/tps65090.h
> index 5989212..804e280 100644
> --- a/include/linux/mfd/tps65090.h
> +++ b/include/linux/mfd/tps65090.h
> @@ -64,10 +64,15 @@ struct tps65090_subdev_info {
>   * struct tps65090_regulator_plat_data
>   *
>   * @reg_init_data: The regulator init data.
> + * @enable_ext_control: Enable extrenal control or not. Only available for
> + *     DCDC1, DCDC2 and DCDC3.
> + * @gpio: Gpio number if external control is enabled and controlled through
> + *     gpio.
>   */
> -
>  struct tps65090_regulator_plat_data {
>  	struct regulator_init_data *reg_init_data;
> +	bool enable_ext_control;
> +	int gpio;
>  };
> 
>  struct tps65090_platform_data {
> --
> 1.7.1.1

Re: [PATCH V2 4/4] regulator: tps65090: add external control support for DCDC

[Mark Brown]

On Tue, Oct 09, 2012 at 04:03:19PM +0530, Venu Byravarasu wrote:
> > -----Original Message-----
> > From: Laxman Dewangan
> > Sent: Tuesday, October 09, 2012 3:19 PM

Please, when replying to mails delete irrelevant text - only quote the
bits of text you're replying to.  This is very helpful to people reading
your reply, it makes it much easier to find the new text especially for
people on mobile devices.

Re: [PATCH V2 4/4] regulator: tps65090: add external control support for DCDC

[Venu Byravarasu]

On Tuesday 09 October 2012 03:32 PM, Laxman Dewangan wrote:
> On Tuesday 09 October 2012 04:03 PM, Venu Byravarasu wrote:
>>> +static struct regulator_ops tps65090_ext_control_ops = {
>>> +};
>> What is the purpose of adding empty structure?
>>
> We will not able to register regulator if there is no ops.
> In regulator register:
>           if (regulator_desc->name == NULL || regulator_desc->ops == NULL)
>                   return ERR_PTR(-EINVAL);
>
If tps65090_ops would be left as is, without renaming it, hope this empty
structure can be removed.

Re: [PATCH V2 4/4] regulator: tps65090: add external control support for DCDC

[Laxman Dewangan]

On Tuesday 09 October 2012 04:41 PM, Venu Byravarasu wrote:
> On Tuesday 09 October 2012 03:32 PM, Laxman Dewangan wrote:
>> On Tuesday 09 October 2012 04:03 PM, Venu Byravarasu wrote:
>>>> +static struct regulator_ops tps65090_ext_control_ops = {
>>>> +};
>>> What is the purpose of adding empty structure?
>>>
>> We will not able to register regulator if there is no ops.
>> In regulator register:
>>            if (regulator_desc->name == NULL || regulator_desc->ops == NULL)
>>                    return ERR_PTR(-EINVAL);
>>
> If tps65090_ops would be left as is, without renaming it, hope this empty
> structure can be removed.
>

When we enabled the external control, the control signal can driver 
either  from gpio or some other mechanism like in our tegra3, the 
control signal come from PMC controller which does not go via GPIO.
The core regulator first check for the gpio and if it is there then it 
will go via the gpiolib to enable/disable. If not then it ask for the 
ops->enable and if I provide these apis then it will endup in the 
regulator driver. I do not want to provide the enable() when external 
control is enabled and no gpios. So in this case:
- External control should be enabled on device registers.
- No enable() and disable() should success.
- is_regualtor_enabled() return true always.

Re: [PATCH V2 4/4] regulator: tps65090: add external control support for DCDC

[Stephen Warren]

On 10/09/2012 04:33 AM, Venu Byravarasu wrote:
> Laxman Dewangan wrote at Tuesday, October 09, 2012 3:19 PM:
>> The TPS65090's DCDC output can also be enable/disable through the
>> external digital input signal. Add support for enable/disable
>> either through register access via I2C or through external
>> control inputs. The external control inputs can be driven through
>> GPIOs also and hence adding support for passing the GPIO number.

>> -static struct regulator_ops tps65090_ops = {
>> -	.enable = regulator_enable_regmap,
>> -	.disable = regulator_disable_regmap,
>> -	.is_enabled = regulator_is_enabled_regmap,
>> +static struct regulator_ops tps65090_ext_control_ops = {
>> +};
> 
> What is the purpose of adding empty structure?

When replying to a patch (or email in general), it's usually a good idea
to delete stuff that you're not replying to. Otherwise, it can be hard
to find the replies since they're buried in a huge long email.

Re: [PATCH V2 0/4] regulator: tps65090: fix regulator registration and add external control support

[Mark Brown]

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

On Tue, Oct 09, 2012 at 03:18:58PM +0530, Laxman Dewangan wrote:
> This patch series fixes the regulator registration issue and add support
> for external control.
> Patch 1: It renames the driver name and regulators to more appropriate.
> Patch 2: Fix the issue with regulator registration.
> Patch 3: Add LDO support.
> Patch 4: Add voltage output level information.
> Patch 5: Add support for extenal control for DCDC.

Applied all four patches, thanks.

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

Re: [PATCH V2 0/4] regulator: tps65090: fix regulator registration and add external control support

[Laxman Dewangan]

On Wednesday 17 October 2012 06:51 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Tue, Oct 09, 2012 at 03:18:58PM +0530, Laxman Dewangan wrote:
>> This patch series fixes the regulator registration issue and add support
>> for external control.
>> Patch 1: It renames the driver name and regulators to more appropriate.
>> Patch 2: Fix the issue with regulator registration.
>> Patch 3: Add LDO support.
>> Patch 4: Add voltage output level information.
>> Patch 5: Add support for extenal control for DCDC.
> Applied all four patches, thanks.

Thank you very much.
Laxman