[PATCH 0/5] regulator: tps65910: fixes/cleanups in dt supports

[PATCH 0/5] regulator: tps65910: fixes/cleanups in dt supports

[Laxman Dewangan]

Fixing the some parameter initialization and cleanup for dt support.
All patches are self explanatory.
- Use small names for the regulator name to match with doc
- initize of_node properly.
- pass proper device structure when registering regulator
- use dvm_*
- Add some more debug print info.

Laxman Dewangan (5):
  regulator: tps65910: use small letter for regulator names
  regulator: tps65910: set of_node for regulator register config
  regulator: tps65910:  use self device for regulator registration
  regulator: tps65910: use devm_* for memory allocation
  regulator: tps65910: add error message in case of failure

 drivers/regulator/tps65910-regulator.c |  165 ++++++++++++++++----------------
 1 files changed, 84 insertions(+), 81 deletions(-)

[PATCH 1/5] regulator: tps65910: use small letter for regulator names

[Laxman Dewangan]

Use the small-letter for names of different regulator to
match the names with dt binding documents.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/tps65910-regulator.c |  104 ++++++++++++++++----------------
 1 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
index 7014843..bd616a0 100644
--- a/drivers/regulator/tps65910-regulator.c
+++ b/drivers/regulator/tps65910-regulator.c
@@ -94,11 +94,11 @@ struct tps_info {
 
 static struct tps_info tps65910_regs[] = {
 	{
-		.name = "VRTC",
+		.name = "vrtc",
 		.enable_time_us = 2200,
 	},
 	{
-		.name = "VIO",
+		.name = "vio",
 		.min_uV = 1500000,
 		.max_uV = 3300000,
 		.n_voltages = ARRAY_SIZE(VIO_VSEL_table),
@@ -106,19 +106,19 @@ static struct tps_info tps65910_regs[] = {
 		.enable_time_us = 350,
 	},
 	{
-		.name = "VDD1",
+		.name = "vdd1",
 		.min_uV = 600000,
 		.max_uV = 4500000,
 		.enable_time_us = 350,
 	},
 	{
-		.name = "VDD2",
+		.name = "vdd2",
 		.min_uV = 600000,
 		.max_uV = 4500000,
 		.enable_time_us = 350,
 	},
 	{
-		.name = "VDD3",
+		.name = "vdd3",
 		.min_uV = 5000000,
 		.max_uV = 5000000,
 		.n_voltages = ARRAY_SIZE(VDD3_VSEL_table),
@@ -126,7 +126,7 @@ static struct tps_info tps65910_regs[] = {
 		.enable_time_us = 200,
 	},
 	{
-		.name = "VDIG1",
+		.name = "vdig1",
 		.min_uV = 1200000,
 		.max_uV = 2700000,
 		.n_voltages = ARRAY_SIZE(VDIG1_VSEL_table),
@@ -134,7 +134,7 @@ static struct tps_info tps65910_regs[] = {
 		.enable_time_us = 100,
 	},
 	{
-		.name = "VDIG2",
+		.name = "vdig2",
 		.min_uV = 1000000,
 		.max_uV = 1800000,
 		.n_voltages = ARRAY_SIZE(VDIG2_VSEL_table),
@@ -142,7 +142,7 @@ static struct tps_info tps65910_regs[] = {
 		.enable_time_us = 100,
 	},
 	{
-		.name = "VPLL",
+		.name = "vpll",
 		.min_uV = 1000000,
 		.max_uV = 2500000,
 		.n_voltages = ARRAY_SIZE(VPLL_VSEL_table),
@@ -150,7 +150,7 @@ static struct tps_info tps65910_regs[] = {
 		.enable_time_us = 100,
 	},
 	{
-		.name = "VDAC",
+		.name = "vdac",
 		.min_uV = 1800000,
 		.max_uV = 2850000,
 		.n_voltages = ARRAY_SIZE(VDAC_VSEL_table),
@@ -158,7 +158,7 @@ static struct tps_info tps65910_regs[] = {
 		.enable_time_us = 100,
 	},
 	{
-		.name = "VAUX1",
+		.name = "vaux1",
 		.min_uV = 1800000,
 		.max_uV = 2850000,
 		.n_voltages = ARRAY_SIZE(VAUX1_VSEL_table),
@@ -166,7 +166,7 @@ static struct tps_info tps65910_regs[] = {
 		.enable_time_us = 100,
 	},
 	{
-		.name = "VAUX2",
+		.name = "vaux2",
 		.min_uV = 1800000,
 		.max_uV = 3300000,
 		.n_voltages = ARRAY_SIZE(VAUX2_VSEL_table),
@@ -174,7 +174,7 @@ static struct tps_info tps65910_regs[] = {
 		.enable_time_us = 100,
 	},
 	{
-		.name = "VAUX33",
+		.name = "vaux33",
 		.min_uV = 1800000,
 		.max_uV = 3300000,
 		.n_voltages = ARRAY_SIZE(VAUX33_VSEL_table),
@@ -182,7 +182,7 @@ static struct tps_info tps65910_regs[] = {
 		.enable_time_us = 100,
 	},
 	{
-		.name = "VMMC",
+		.name = "vmmc",
 		.min_uV = 1800000,
 		.max_uV = 3300000,
 		.n_voltages = ARRAY_SIZE(VMMC_VSEL_table),
@@ -193,11 +193,11 @@ static struct tps_info tps65910_regs[] = {
 
 static struct tps_info tps65911_regs[] = {
 	{
-		.name = "VRTC",
+		.name = "vrtc",
 		.enable_time_us = 2200,
 	},
 	{
-		.name = "VIO",
+		.name = "vio",
 		.min_uV = 1500000,
 		.max_uV = 3300000,
 		.n_voltages = ARRAY_SIZE(VIO_VSEL_table),
@@ -205,77 +205,77 @@ static struct tps_info tps65911_regs[] = {
 		.enable_time_us = 350,
 	},
 	{
-		.name = "VDD1",
+		.name = "vdd1",
 		.min_uV = 600000,
 		.max_uV = 4500000,
 		.n_voltages = 73,
 		.enable_time_us = 350,
 	},
 	{
-		.name = "VDD2",
+		.name = "vdd2",
 		.min_uV = 600000,
 		.max_uV = 4500000,
 		.n_voltages = 73,
 		.enable_time_us = 350,
 	},
 	{
-		.name = "VDDCTRL",
+		.name = "vddctrl",
 		.min_uV = 600000,
 		.max_uV = 1400000,
 		.n_voltages = 65,
 		.enable_time_us = 900,
 	},
 	{
-		.name = "LDO1",
+		.name = "ldo1",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
 		.n_voltages = 47,
 		.enable_time_us = 420,
 	},
 	{
-		.name = "LDO2",
+		.name = "ldo2",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
 		.n_voltages = 47,
 		.enable_time_us = 420,
 	},
 	{
-		.name = "LDO3",
+		.name = "ldo3",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
 		.n_voltages = 24,
 		.enable_time_us = 230,
 	},
 	{
-		.name = "LDO4",
+		.name = "ldo4",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
 		.n_voltages = 47,
 		.enable_time_us = 230,
 	},
 	{
-		.name = "LDO5",
+		.name = "ldo5",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
 		.n_voltages = 24,
 		.enable_time_us = 230,
 	},
 	{
-		.name = "LDO6",
+		.name = "ldo6",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
 		.n_voltages = 24,
 		.enable_time_us = 230,
 	},
 	{
-		.name = "LDO7",
+		.name = "ldo7",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
 		.n_voltages = 24,
 		.enable_time_us = 230,
 	},
 	{
-		.name = "LDO8",
+		.name = "ldo8",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
 		.n_voltages = 24,
@@ -1039,35 +1039,35 @@ static int tps65910_set_ext_sleep_config(struct tps65910_reg *pmic,
 #ifdef CONFIG_OF
 
 static struct of_regulator_match tps65910_matches[] = {
-	{ .name = "VRTC",	.driver_data = (void *) &tps65910_regs[0] },
-	{ .name = "VIO",	.driver_data = (void *) &tps65910_regs[1] },
-	{ .name = "VDD1",	.driver_data = (void *) &tps65910_regs[2] },
-	{ .name = "VDD2",	.driver_data = (void *) &tps65910_regs[3] },
-	{ .name = "VDD3",	.driver_data = (void *) &tps65910_regs[4] },
-	{ .name = "VDIG1",	.driver_data = (void *) &tps65910_regs[5] },
-	{ .name = "VDIG2",	.driver_data = (void *) &tps65910_regs[6] },
-	{ .name = "VPLL",	.driver_data = (void *) &tps65910_regs[7] },
-	{ .name = "VDAC",	.driver_data = (void *) &tps65910_regs[8] },
-	{ .name = "VAUX1",	.driver_data = (void *) &tps65910_regs[9] },
-	{ .name = "VAUX2",	.driver_data = (void *) &tps65910_regs[10] },
-	{ .name = "VAUX33",	.driver_data = (void *) &tps65910_regs[11] },
-	{ .name = "VMMC",	.driver_data = (void *) &tps65910_regs[12] },
+	{ .name = "vrtc",	.driver_data = (void *) &tps65910_regs[0] },
+	{ .name = "vio",	.driver_data = (void *) &tps65910_regs[1] },
+	{ .name = "vdd1",	.driver_data = (void *) &tps65910_regs[2] },
+	{ .name = "vdd2",	.driver_data = (void *) &tps65910_regs[3] },
+	{ .name = "vdd3",	.driver_data = (void *) &tps65910_regs[4] },
+	{ .name = "vdig1",	.driver_data = (void *) &tps65910_regs[5] },
+	{ .name = "vdig2",	.driver_data = (void *) &tps65910_regs[6] },
+	{ .name = "vpll",	.driver_data = (void *) &tps65910_regs[7] },
+	{ .name = "vdac",	.driver_data = (void *) &tps65910_regs[8] },
+	{ .name = "vaux1",	.driver_data = (void *) &tps65910_regs[9] },
+	{ .name = "vaux2",	.driver_data = (void *) &tps65910_regs[10] },
+	{ .name = "vaux33",	.driver_data = (void *) &tps65910_regs[11] },
+	{ .name = "vmmc",	.driver_data = (void *) &tps65910_regs[12] },
 };
 
 static struct of_regulator_match tps65911_matches[] = {
-	{ .name = "VRTC",	.driver_data = (void *) &tps65911_regs[0] },
-	{ .name = "VIO",	.driver_data = (void *) &tps65911_regs[1] },
-	{ .name = "VDD1",	.driver_data = (void *) &tps65911_regs[2] },
-	{ .name = "VDD2",	.driver_data = (void *) &tps65911_regs[3] },
-	{ .name = "VDDCTRL",	.driver_data = (void *) &tps65911_regs[4] },
-	{ .name = "LDO1",	.driver_data = (void *) &tps65911_regs[5] },
-	{ .name = "LDO2",	.driver_data = (void *) &tps65911_regs[6] },
-	{ .name = "LDO3",	.driver_data = (void *) &tps65911_regs[7] },
-	{ .name = "LDO4",	.driver_data = (void *) &tps65911_regs[8] },
-	{ .name = "LDO5",	.driver_data = (void *) &tps65911_regs[9] },
-	{ .name = "LDO6",	.driver_data = (void *) &tps65911_regs[10] },
-	{ .name = "LDO7",	.driver_data = (void *) &tps65911_regs[11] },
-	{ .name = "LDO8",	.driver_data = (void *) &tps65911_regs[12] },
+	{ .name = "vrtc",	.driver_data = (void *) &tps65911_regs[0] },
+	{ .name = "vio",	.driver_data = (void *) &tps65911_regs[1] },
+	{ .name = "vdd1",	.driver_data = (void *) &tps65911_regs[2] },
+	{ .name = "vdd2",	.driver_data = (void *) &tps65911_regs[3] },
+	{ .name = "vddctrl",	.driver_data = (void *) &tps65911_regs[4] },
+	{ .name = "ldo1",	.driver_data = (void *) &tps65911_regs[5] },
+	{ .name = "ldo2",	.driver_data = (void *) &tps65911_regs[6] },
+	{ .name = "ldo3",	.driver_data = (void *) &tps65911_regs[7] },
+	{ .name = "ldo4",	.driver_data = (void *) &tps65911_regs[8] },
+	{ .name = "ldo5",	.driver_data = (void *) &tps65911_regs[9] },
+	{ .name = "ldo6",	.driver_data = (void *) &tps65911_regs[10] },
+	{ .name = "ldo7",	.driver_data = (void *) &tps65911_regs[11] },
+	{ .name = "ldo8",	.driver_data = (void *) &tps65911_regs[12] },
 };
 
 static struct tps65910_board *tps65910_parse_dt_reg_data(
-- 
1.7.1.1

[PATCH 2/5] regulator: tps65910: set of_node for regulator register config

[Laxman Dewangan]

set of_node parameter for regulator registration configuration
parameter. This node contains the regulator related device node
information for regulator which need to be register.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/tps65910-regulator.c |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
index bd616a0..dd0e2cb 100644
--- a/drivers/regulator/tps65910-regulator.c
+++ b/drivers/regulator/tps65910-regulator.c
@@ -1071,7 +1071,7 @@ static struct of_regulator_match tps65911_matches[] = {
 };
 
 static struct tps65910_board *tps65910_parse_dt_reg_data(
-				struct platform_device *pdev)
+	struct platform_device *pdev, struct device_node **regulators_node)
 {
 	struct tps65910_board *pmic_plat_data;
 	struct tps65910 *tps65910 = dev_get_drvdata(pdev->dev.parent);
@@ -1090,6 +1090,12 @@ static struct tps65910_board *tps65910_parse_dt_reg_data(
 	}
 
 	regulators = of_find_node_by_name(np, "regulators");
+	if (!regulators) {
+		dev_err(&pdev->dev, "regulator node not found\n");
+		return NULL;
+	}
+
+	*regulators_node = regulators;
 
 	switch (tps65910_chip_id(tps65910)) {
 	case TPS65910:
@@ -1129,8 +1135,9 @@ static struct tps65910_board *tps65910_parse_dt_reg_data(
 }
 #else
 static inline struct tps65910_board *tps65910_parse_dt_reg_data(
-				struct platform_device *pdev)
+	struct platform_device *pdev, struct device_node **regulators_node)
 {
+	*regulators_node = NULL;
 	return 0;
 }
 #endif
@@ -1145,10 +1152,11 @@ static __devinit int tps65910_probe(struct platform_device *pdev)
 	struct tps65910_reg *pmic;
 	struct tps65910_board *pmic_plat_data;
 	int i, err;
+	struct device_node *reg_node = NULL;
 
 	pmic_plat_data = dev_get_platdata(tps65910->dev);
 	if (!pmic_plat_data && tps65910->dev->of_node)
-		pmic_plat_data = tps65910_parse_dt_reg_data(pdev);
+		pmic_plat_data = tps65910_parse_dt_reg_data(pdev, &reg_node);
 
 	if (!pmic_plat_data)
 		return -EINVAL;
@@ -1257,10 +1265,9 @@ static __devinit int tps65910_probe(struct platform_device *pdev)
 		config.driver_data = pmic;
 		config.regmap = tps65910->regmap;
 
-#ifdef CONFIG_OF
-		config.of_node = of_find_node_by_name(tps65910->dev->of_node,
+		if (reg_node)
+			config.of_node = of_find_node_by_name(reg_node,
 							info->name);
-#endif
 
 		rdev = regulator_register(&pmic->desc[i], &config);
 		if (IS_ERR(rdev)) {
-- 
1.7.1.1

[PATCH 3/5] regulator: tps65910: use self device for regulator registration

[Laxman Dewangan]

Use self device for regulator registration instead of parent
device.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/tps65910-regulator.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
index dd0e2cb..72942c2 100644
--- a/drivers/regulator/tps65910-regulator.c
+++ b/drivers/regulator/tps65910-regulator.c
@@ -1111,7 +1111,7 @@ static struct tps65910_board *tps65910_parse_dt_reg_data(
 		return NULL;
 	}
 
-	ret = of_regulator_match(pdev->dev.parent, regulators, matches, count);
+	ret = of_regulator_match(&pdev->dev, regulators, matches, count);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Error parsing regulator init data: %d\n",
 			ret);
@@ -1260,7 +1260,7 @@ static __devinit int tps65910_probe(struct platform_device *pdev)
 		pmic->desc[i].enable_reg = pmic->get_ctrl_reg(i);
 		pmic->desc[i].enable_mask = TPS65910_SUPPLY_STATE_ENABLED;
 
-		config.dev = tps65910->dev;
+		config.dev = &pdev->dev;
 		config.init_data = reg_data;
 		config.driver_data = pmic;
 		config.regmap = tps65910->regmap;
-- 
1.7.1.1

[PATCH 4/5] regulator: tps65910: use devm_* for memory allocation

[Laxman Dewangan]

use the devm_* apis for memory allocation.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/tps65910-regulator.c |   27 +++++++++------------------
 1 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
index 72942c2..5a8ad4e 100644
--- a/drivers/regulator/tps65910-regulator.c
+++ b/drivers/regulator/tps65910-regulator.c
@@ -1191,25 +1191,25 @@ static __devinit int tps65910_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	pmic->desc = kcalloc(pmic->num_regulators,
+	pmic->desc = devm_kzalloc(&pdev->dev, pmic->num_regulators *
 			sizeof(struct regulator_desc), GFP_KERNEL);
 	if (!pmic->desc) {
-		err = -ENOMEM;
-		goto err_out;
+		dev_err(&pdev->dev, "Memory alloc fails for desc\n");
+		return -ENOMEM;
 	}
 
-	pmic->info = kcalloc(pmic->num_regulators,
+	pmic->info = devm_kzalloc(&pdev->dev, pmic->num_regulators *
 			sizeof(struct tps_info *), GFP_KERNEL);
 	if (!pmic->info) {
-		err = -ENOMEM;
-		goto err_free_desc;
+		dev_err(&pdev->dev, "Memory alloc fails for info\n");
+		return -ENOMEM;
 	}
 
-	pmic->rdev = kcalloc(pmic->num_regulators,
+	pmic->rdev = devm_kzalloc(&pdev->dev, pmic->num_regulators *
 			sizeof(struct regulator_dev *), GFP_KERNEL);
 	if (!pmic->rdev) {
-		err = -ENOMEM;
-		goto err_free_info;
+		dev_err(&pdev->dev, "Memory alloc fails for rdev\n");
+		return -ENOMEM;
 	}
 
 	for (i = 0; i < pmic->num_regulators && i < TPS65910_NUM_REGS;
@@ -1286,12 +1286,6 @@ static __devinit int tps65910_probe(struct platform_device *pdev)
 err_unregister_regulator:
 	while (--i >= 0)
 		regulator_unregister(pmic->rdev[i]);
-	kfree(pmic->rdev);
-err_free_info:
-	kfree(pmic->info);
-err_free_desc:
-	kfree(pmic->desc);
-err_out:
 	return err;
 }
 
@@ -1303,9 +1297,6 @@ static int __devexit tps65910_remove(struct platform_device *pdev)
 	for (i = 0; i < pmic->num_regulators; i++)
 		regulator_unregister(pmic->rdev[i]);
 
-	kfree(pmic->rdev);
-	kfree(pmic->info);
-	kfree(pmic->desc);
 	return 0;
 }
 
-- 
1.7.1.1

[PATCH 5/5] regulator: tps65910: add error message in case of failure

[Laxman Dewangan]

Prints error message whenever there is failure on allocating
resource. Also used the dev_* to print messages instead of pr_*

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/tps65910-regulator.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
index 5a8ad4e..560282c 100644
--- a/drivers/regulator/tps65910-regulator.c
+++ b/drivers/regulator/tps65910-regulator.c
@@ -1107,7 +1107,7 @@ static struct tps65910_board *tps65910_parse_dt_reg_data(
 		matches = tps65911_matches;
 		break;
 	default:
-		pr_err("Invalid tps chip version\n");
+		dev_err(&pdev->dev, "Invalid tps chip version\n");
 		return NULL;
 	}
 
@@ -1158,12 +1158,16 @@ static __devinit int tps65910_probe(struct platform_device *pdev)
 	if (!pmic_plat_data && tps65910->dev->of_node)
 		pmic_plat_data = tps65910_parse_dt_reg_data(pdev, &reg_node);
 
-	if (!pmic_plat_data)
+	if (!pmic_plat_data) {
+		dev_err(&pdev->dev, "Platform data not found\n");
 		return -EINVAL;
+	}
 
 	pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL);
-	if (!pmic)
+	if (!pmic) {
+		dev_err(&pdev->dev, "Memory allocation failed for pmic\n");
 		return -ENOMEM;
+	}
 
 	mutex_init(&pmic->mutex);
 	pmic->mfd = tps65910;
@@ -1187,7 +1191,7 @@ static __devinit int tps65910_probe(struct platform_device *pdev)
 		info = tps65911_regs;
 		break;
 	default:
-		pr_err("Invalid tps chip version\n");
+		dev_err(&pdev->dev, "Invalid tps chip version\n");
 		return -ENODEV;
 	}
 
-- 
1.7.1.1

Re: [PATCH 1/5] regulator: tps65910: use small letter for regulator names

[Mark Brown]

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

On Sat, May 19, 2012 at 08:04:06PM +0530, Laxman Dewangan wrote:
> Use the small-letter for names of different regulator to
> match the names with dt binding documents.

Applied, thanks.

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

Re: [PATCH 4/5] regulator: tps65910: use devm_* for memory allocation

[Mark Brown]

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

On Sat, May 19, 2012 at 08:04:09PM +0530, Laxman Dewangan wrote:
> use the devm_* apis for memory allocation.

Applied, thanks.

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

Re: [PATCH 2/5] regulator: tps65910: set of_node for regulator register config

[Mark Brown]

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

On Sat, May 19, 2012 at 08:04:07PM +0530, Laxman Dewangan wrote:

> set of_node parameter for regulator registration configuration
> parameter. This node contains the regulator related device node
> information for regulator which need to be register.

This isn't adding setting of the of_node...

> -#ifdef CONFIG_OF
> -		config.of_node = of_find_node_by_name(tps65910->dev->of_node,
> +		if (reg_node)
> +			config.of_node = of_find_node_by_name(reg_node,
>  							info->name);
> -#endif

...it's changing the value that's being configured.  The changelog
doesn't explain why this is being done.

Also, why is the ifdef being removed - won't that break the build?

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

Re: [PATCH 3/5] regulator: tps65910: use self device for regulator registration

[Mark Brown]

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

On Sat, May 19, 2012 at 08:04:08PM +0530, Laxman Dewangan wrote:

> Use self device for regulator registration instead of parent
> device.

What's the motivation behind this change?  I'd expect the MFD device
(which is the actual physical device) to make things like dev_ prints
have an actual physical device which seems a bit more user friendly.  
Making this change will also mean that things like regulator to
regulator supplies are less obvious on non-DT systems as they'll wind up
having to specify the subdevice, and on DT systems it'll change the
binding from being for the parent device to the subdevice.

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

Re: [PATCH 2/5] regulator: tps65910: set of_node for regulator register config

[Laxman Dewangan]

On Saturday 19 May 2012 11:18 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Sat, May 19, 2012 at 08:04:07PM +0530, Laxman Dewangan wrote:
>
>
>
>> -#ifdef CONFIG_OF
>> -		config.of_node = of_find_node_by_name(tps65910->dev->of_node,
>> +		if (reg_node)
>> +			config.of_node = of_find_node_by_name(reg_node,
>>   							info->name);
>> -#endif
> ...it's changing the value that's being configured.  The changelog
> doesn't explain why this is being done.
>
> Also, why is the ifdef being removed - won't that break the build?
Yaah there is no function for non-dt case and should we add that in of.h?
static inline struct device_node *of_find_node_by_name(struct 
device_node *from,
         const char *name)
{
     return NULL;
}

In this case it will not need to have #ifdef CONFIG_OF in the client code?

Re: [PATCH 2/5] regulator: tps65910: set of_node for regulator register config

[Mark Brown]

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

On Sat, May 19, 2012 at 11:49:53PM +0530, Laxman Dewangan wrote:

> In this case it will not need to have #ifdef CONFIG_OF in the client code?

Right, we'll be able to skip it.

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

Re: [PATCH 5/5] regulator: tps65910: add error message in case of failure

[Mark Brown]

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

On Sat, May 19, 2012 at 08:04:10PM +0530, Laxman Dewangan wrote:
> Prints error message whenever there is failure on allocating
> resource. Also used the dev_* to print messages instead of pr_*

This is fine but it depends on patch 2 so wouldn't apply.  I generally
recommend putting this sort of cleanup earlier on in the series so that
it can be applied even if there's some problem with the more substantial
changes later on.

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