[PATCH] mfd/regulator: tps65217: Move regulator plat data handling to regulator

[PATCH] mfd/regulator: tps65217: Move regulator plat data handling to regulator

[AnilKumar Ch]

Regulator platform data handling was mistakenly added to MFD
driver. So we will see build errors if we compile MFD drivers
without CONFIG_REGULATOR. This patch moves regulator platform
data handling from TPS65217 MFD driver to regulator driver.

This makes MFD driver independent of REGULATOR framework so
build error is fixed if CONFIG_REGULATOR is not set.

drivers/built-in.o: In function `tps65217_probe':
tps65217.c:(.devinit.text+0x13e37): undefined reference
to `of_regulator_match'

This patch is based on linux-next (20120720) tree

Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
 drivers/mfd/tps65217.c                 |  130 +++++++++++---------------------
 drivers/regulator/tps65217-regulator.c |  124 ++++++++++++++++++++++++++----
 include/linux/mfd/tps65217.h           |   12 ++-
 3 files changed, 161 insertions(+), 105 deletions(-)

diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
index 61c097a..3bc2744 100644
--- a/drivers/mfd/tps65217.c
+++ b/drivers/mfd/tps65217.c
@@ -24,11 +24,18 @@
 #include <linux/slab.h>
 #include <linux/regmap.h>
 #include <linux/err.h>
-#include <linux/regulator/of_regulator.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <linux/mfd/core.h>
 #include <linux/mfd/tps65217.h>
 
+static struct mfd_cell tps65217s[] = {
+	{
+		.name = "tps65217-pmic",
+	},
+};
+
 /**
  * tps65217_reg_read: Read a single tps65217 register.
  *
@@ -133,83 +140,48 @@ int tps65217_clear_bits(struct tps65217 *tps, unsigned int reg,
 }
 EXPORT_SYMBOL_GPL(tps65217_clear_bits);
 
-#ifdef CONFIG_OF
-static struct of_regulator_match reg_matches[] = {
-	{ .name = "dcdc1", .driver_data = (void *)TPS65217_DCDC_1 },
-	{ .name = "dcdc2", .driver_data = (void *)TPS65217_DCDC_2 },
-	{ .name = "dcdc3", .driver_data = (void *)TPS65217_DCDC_3 },
-	{ .name = "ldo1", .driver_data = (void *)TPS65217_LDO_1 },
-	{ .name = "ldo2", .driver_data = (void *)TPS65217_LDO_2 },
-	{ .name = "ldo3", .driver_data = (void *)TPS65217_LDO_3 },
-	{ .name = "ldo4", .driver_data = (void *)TPS65217_LDO_4 },
-};
-
-static struct tps65217_board *tps65217_parse_dt(struct i2c_client *client)
-{
-	struct device_node *node = client->dev.of_node;
-	struct tps65217_board *pdata;
-	struct device_node *regs;
-	int count = ARRAY_SIZE(reg_matches);
-	int ret, i;
-
-	regs = of_find_node_by_name(node, "regulators");
-	if (!regs)
-		return NULL;
-
-	ret = of_regulator_match(&client->dev, regs, reg_matches, count);
-	of_node_put(regs);
-	if ((ret < 0) || (ret > count))
-		return NULL;
-
-	count = ret;
-	pdata = devm_kzalloc(&client->dev, count * sizeof(*pdata), GFP_KERNEL);
-	if (!pdata)
-		return NULL;
-
-	for (i = 0; i < count; i++) {
-		if (!reg_matches[i].init_data || !reg_matches[i].of_node)
-			continue;
-
-		pdata->tps65217_init_data[i] = reg_matches[i].init_data;
-		pdata->of_node[i] = reg_matches[i].of_node;
-	}
-
-	return pdata;
-}
-
-static struct of_device_id tps65217_of_match[] = {
-	{ .compatible = "ti,tps65217", },
-	{ },
-};
-#else
-static struct tps65217_board *tps65217_parse_dt(struct i2c_client *client)
-{
-	return NULL;
-}
-#endif
-
 static struct regmap_config tps65217_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
 };
 
+static const struct of_device_id tps65217_of_match[] = {
+	{ .compatible = "ti,tps65217", .data = (void *)TPS65217 },
+	{ /* sentinel */ },
+};
+
 static int __devinit tps65217_probe(struct i2c_client *client,
 				const struct i2c_device_id *ids)
 {
 	struct tps65217 *tps;
-	struct regulator_init_data *reg_data;
-	struct tps65217_board *pdata = client->dev.platform_data;
-	int i, ret;
 	unsigned int version;
+	unsigned int chip_id = ids->driver_data;
+	const struct of_device_id *match;
+	int ret;
 
-	if (!pdata && client->dev.of_node)
-		pdata = tps65217_parse_dt(client);
+	if (client->dev.of_node) {
+		match = of_match_device(tps65217_of_match, &client->dev);
+		if (!match) {
+			dev_err(&client->dev,
+				"Failed to find matching dt id\n");
+			return -EINVAL;
+		}
+		chip_id = (unsigned int)match->data;
+	}
+
+	if (!chip_id) {
+		dev_err(&client->dev, "id is null.\n");
+		return -ENODEV;
+	}
 
 	tps = devm_kzalloc(&client->dev, sizeof(*tps), GFP_KERNEL);
 	if (!tps)
 		return -ENOMEM;
 
-	tps->pdata = pdata;
+	i2c_set_clientdata(client, tps);
+	tps->dev = &client->dev;
+	tps->id = chip_id;
+
 	tps->regmap = devm_regmap_init_i2c(client, &tps65217_regmap_config);
 	if (IS_ERR(tps->regmap)) {
 		ret = PTR_ERR(tps->regmap);
@@ -218,8 +190,12 @@ static int __devinit tps65217_probe(struct i2c_client *client,
 		return ret;
 	}
 
-	i2c_set_clientdata(client, tps);
-	tps->dev = &client->dev;
+	ret = mfd_add_devices(tps->dev, -1, tps65217s,
+					ARRAY_SIZE(tps65217s), NULL, 0);
+	if (ret < 0) {
+		dev_err(tps->dev, "mfd_add_devices failed: %d\n", ret);
+		return ret;
+	}
 
 	ret = tps65217_reg_read(tps, TPS65217_REG_CHIPID, &version);
 	if (ret < 0) {
@@ -232,41 +208,21 @@ static int __devinit tps65217_probe(struct i2c_client *client,
 			(version & TPS65217_CHIPID_CHIP_MASK) >> 4,
 			version & TPS65217_CHIPID_REV_MASK);
 
-	for (i = 0; i < TPS65217_NUM_REGULATOR; i++) {
-		struct platform_device *pdev;
-
-		pdev = platform_device_alloc("tps65217-pmic", i);
-		if (!pdev) {
-			dev_err(tps->dev, "Cannot create regulator %d\n", i);
-			continue;
-		}
-
-		pdev->dev.parent = tps->dev;
-		pdev->dev.of_node = pdata->of_node[i];
-		reg_data = pdata->tps65217_init_data[i];
-		platform_device_add_data(pdev, reg_data, sizeof(*reg_data));
-		tps->regulator_pdev[i] = pdev;
-
-		platform_device_add(pdev);
-	}
-
 	return 0;
 }
 
 static int __devexit tps65217_remove(struct i2c_client *client)
 {
 	struct tps65217 *tps = i2c_get_clientdata(client);
-	int i;
 
-	for (i = 0; i < TPS65217_NUM_REGULATOR; i++)
-		platform_device_unregister(tps->regulator_pdev[i]);
+	mfd_remove_devices(tps->dev);
 
 	return 0;
 }
 
 static const struct i2c_device_id tps65217_id_table[] = {
-	{"tps65217", 0xF0},
-	{/* end of list */}
+	{"tps65217", TPS65217},
+	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(i2c, tps65217_id_table);
 
diff --git a/drivers/regulator/tps65217-regulator.c b/drivers/regulator/tps65217-regulator.c
index 6caa222..9730f1d 100644
--- a/drivers/regulator/tps65217-regulator.c
+++ b/drivers/regulator/tps65217-regulator.c
@@ -22,6 +22,7 @@
 #include <linux/err.h>
 #include <linux/platform_device.h>
 
+#include <linux/regulator/of_regulator.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
 #include <linux/mfd/tps65217.h>
@@ -281,37 +282,130 @@ static const struct regulator_desc regulators[] = {
 			   NULL),
 };
 
+#ifdef CONFIG_OF
+static struct of_regulator_match reg_matches[] = {
+	{ .name = "dcdc1", .driver_data = (void *)TPS65217_DCDC_1 },
+	{ .name = "dcdc2", .driver_data = (void *)TPS65217_DCDC_2 },
+	{ .name = "dcdc3", .driver_data = (void *)TPS65217_DCDC_3 },
+	{ .name = "ldo1", .driver_data = (void *)TPS65217_LDO_1 },
+	{ .name = "ldo2", .driver_data = (void *)TPS65217_LDO_2 },
+	{ .name = "ldo3", .driver_data = (void *)TPS65217_LDO_3 },
+	{ .name = "ldo4", .driver_data = (void *)TPS65217_LDO_4 },
+};
+
+static struct tps65217_board *tps65217_parse_dt(struct platform_device *pdev)
+{
+	struct tps65217 *tps = dev_get_drvdata(pdev->dev.parent);
+	struct device_node *node = tps->dev->of_node;
+	struct tps65217_board *pdata;
+	struct device_node *regs;
+	int i, count;
+
+	regs = of_find_node_by_name(node, "regulators");
+	if (!regs)
+		return NULL;
+
+	count = of_regulator_match(pdev->dev.parent, regs,
+				reg_matches, TPS65217_NUM_REGULATOR);
+	of_node_put(regs);
+	if ((count < 0) || (count > TPS65217_NUM_REGULATOR))
+		return NULL;
+
+	pdata = devm_kzalloc(&pdev->dev, count * sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	for (i = 0; i < count; i++) {
+		if (!reg_matches[i].init_data || !reg_matches[i].of_node)
+			continue;
+
+		pdata->tps65217_init_data[i] = reg_matches[i].init_data;
+		pdata->of_node[i] = reg_matches[i].of_node;
+	}
+
+	return pdata;
+}
+#else
+static struct tps65217_board *tps65217_parse_dt(struct platform_device *pdev)
+{
+	return NULL;
+}
+#endif
+
 static int __devinit tps65217_regulator_probe(struct platform_device *pdev)
 {
+	struct tps65217 *tps = dev_get_drvdata(pdev->dev.parent);
+	struct tps65217_board *pdata = dev_get_platdata(tps->dev);
+	struct regulator_init_data *reg_data;
 	struct regulator_dev *rdev;
-	struct tps65217 *tps;
-	struct tps_info *info = &tps65217_pmic_regs[pdev->id];
 	struct regulator_config config = { };
+	int i, ret;
 
-	/* Already set by core driver */
-	tps = dev_to_tps65217(pdev->dev.parent);
-	tps->info[pdev->id] = info;
+	if (tps->dev->of_node)
+		pdata = tps65217_parse_dt(pdev);
 
-	config.dev = &pdev->dev;
-	config.of_node = pdev->dev.of_node;
-	config.init_data = pdev->dev.platform_data;
-	config.driver_data = tps;
+	if (!pdata) {
+		dev_err(&pdev->dev, "Platform data not found\n");
+		return -EINVAL;
+	}
 
-	rdev = regulator_register(&regulators[pdev->id], &config);
-	if (IS_ERR(rdev))
-		return PTR_ERR(rdev);
+	if (tps65217_chip_id(tps) != TPS65217) {
+		dev_err(&pdev->dev, "Invalid tps chip version\n");
+		return -ENODEV;
+	}
 
-	platform_set_drvdata(pdev, rdev);
+	platform_set_drvdata(pdev, tps);
 
+	for (i = 0; i < TPS65217_NUM_REGULATOR; i++) {
+
+		reg_data = pdata->tps65217_init_data[i];
+
+		/*
+		 * Regulator API handles empty constraints but not NULL
+		 * constraints
+		 */
+		if (!reg_data)
+			continue;
+
+		/* Register the regulators */
+		tps->info[i] = &tps65217_pmic_regs[i];
+
+		config.dev = tps->dev;
+		config.init_data = reg_data;
+		config.driver_data = tps;
+		config.regmap = tps->regmap;
+		if (tps->dev->of_node)
+			config.of_node = pdata->of_node[i];
+
+		rdev = regulator_register(&regulators[i], &config);
+		if (IS_ERR(rdev)) {
+			dev_err(tps->dev, "failed to register %s regulator\n",
+				pdev->name);
+			ret = PTR_ERR(rdev);
+			goto err_unregister_regulator;
+		}
+
+		/* Save regulator for cleanup */
+		tps->rdev[i] = rdev;
+	}
 	return 0;
+
+err_unregister_regulator:
+	while (--i >= 0)
+		regulator_unregister(tps->rdev[i]);
+
+	return ret;
 }
 
 static int __devexit tps65217_regulator_remove(struct platform_device *pdev)
 {
-	struct regulator_dev *rdev = platform_get_drvdata(pdev);
+	struct tps65217 *tps = platform_get_drvdata(pdev);
+	unsigned int i;
+
+	for (i = 0; i < TPS65217_NUM_REGULATOR; i++)
+		regulator_unregister(tps->rdev[i]);
 
 	platform_set_drvdata(pdev, NULL);
-	regulator_unregister(rdev);
 
 	return 0;
 }
diff --git a/include/linux/mfd/tps65217.h b/include/linux/mfd/tps65217.h
index 12c0687..7cd83d8 100644
--- a/include/linux/mfd/tps65217.h
+++ b/include/linux/mfd/tps65217.h
@@ -22,6 +22,9 @@
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
 
+/* TPS chip id list */
+#define TPS65217			0xF0
+
 /* I2C ID for TPS65217 part */
 #define TPS65217_I2C_ID			0x24
 
@@ -248,13 +251,11 @@ struct tps_info {
 struct tps65217 {
 	struct device *dev;
 	struct tps65217_board *pdata;
+	unsigned int id;
 	struct regulator_desc desc[TPS65217_NUM_REGULATOR];
 	struct regulator_dev *rdev[TPS65217_NUM_REGULATOR];
 	struct tps_info *info[TPS65217_NUM_REGULATOR];
 	struct regmap *regmap;
-
-	/* Client devices */
-	struct platform_device *regulator_pdev[TPS65217_NUM_REGULATOR];
 };
 
 static inline struct tps65217 *dev_to_tps65217(struct device *dev)
@@ -262,6 +263,11 @@ static inline struct tps65217 *dev_to_tps65217(struct device *dev)
 	return dev_get_drvdata(dev);
 }
 
+static inline int tps65217_chip_id(struct tps65217 *tps65217)
+{
+	return tps65217->id;
+}
+
 int tps65217_reg_read(struct tps65217 *tps, unsigned int reg,
 					unsigned int *val);
 int tps65217_reg_write(struct tps65217 *tps, unsigned int reg,
-- 
1.7.0.4

Re: [PATCH] mfd/regulator: tps65217: Move regulator plat data handling to regulator

[Samuel Ortiz]

Hi Anilkumar,

On Fri, Jul 20, 2012 at 03:00:01PM +0530, AnilKumar Ch wrote:
> Regulator platform data handling was mistakenly added to MFD
> driver. So we will see build errors if we compile MFD drivers
> without CONFIG_REGULATOR. This patch moves regulator platform
> data handling from TPS65217 MFD driver to regulator driver.
> 
> This makes MFD driver independent of REGULATOR framework so
> build error is fixed if CONFIG_REGULATOR is not set.
> 
> drivers/built-in.o: In function `tps65217_probe':
> tps65217.c:(.devinit.text+0x13e37): undefined reference
> to `of_regulator_match'
> 
> This patch is based on linux-next (20120720) tree
> 
> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> ---
>  drivers/mfd/tps65217.c                 |  130 +++++++++++---------------------
>  drivers/regulator/tps65217-regulator.c |  124 ++++++++++++++++++++++++++----
>  include/linux/mfd/tps65217.h           |   12 ++-
>  3 files changed, 161 insertions(+), 105 deletions(-)
It doesn't apply to my for-next branch. Could you please re-send this one
after the merge window is closed ? I'll push that as part of the MFD fixes for
3.6.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

RE: [PATCH] mfd/regulator: tps65217: Move regulator plat data handling to regulator

[AnilKumar, Chimata]

Hi Samuel,

On Fri, Jul 27, 2012 at 18:18:17, Samuel Ortiz wrote:
> Hi Anilkumar,
> 
> On Fri, Jul 20, 2012 at 03:00:01PM +0530, AnilKumar Ch wrote:
> > Regulator platform data handling was mistakenly added to MFD
> > driver. So we will see build errors if we compile MFD drivers
> > without CONFIG_REGULATOR. This patch moves regulator platform
> > data handling from TPS65217 MFD driver to regulator driver.
> > 
> > This makes MFD driver independent of REGULATOR framework so
> > build error is fixed if CONFIG_REGULATOR is not set.
> > 
> > drivers/built-in.o: In function `tps65217_probe':
> > tps65217.c:(.devinit.text+0x13e37): undefined reference
> > to `of_regulator_match'
> > 
> > This patch is based on linux-next (20120720) tree
> > 
> > Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> > ---
> >  drivers/mfd/tps65217.c                 |  130 +++++++++++---------------------
> >  drivers/regulator/tps65217-regulator.c |  124 ++++++++++++++++++++++++++----
> >  include/linux/mfd/tps65217.h           |   12 ++-
> >  3 files changed, 161 insertions(+), 105 deletions(-)
> It doesn't apply to my for-next branch. Could you please re-send this one
> after the merge window is closed ? I'll push that as part of the MFD fixes for
> 3.6.
> 

I fetch regulator tree and applied on top of for-next branch and it is cleanly
applied.

Regards
AnilKumar

RE: [PATCH] mfd/regulator: tps65217: Move regulator plat data handling to regulator

[AnilKumar, Chimata]

Hi Samuel,

On Tue, Jul 31, 2012 at 20:00:21, AnilKumar, Chimata wrote:
> Hi Samuel,
> 
> On Fri, Jul 27, 2012 at 18:18:17, Samuel Ortiz wrote:
> > Hi Anilkumar,
> > 
> > On Fri, Jul 20, 2012 at 03:00:01PM +0530, AnilKumar Ch wrote:
> > > Regulator platform data handling was mistakenly added to MFD
> > > driver. So we will see build errors if we compile MFD drivers
> > > without CONFIG_REGULATOR. This patch moves regulator platform
> > > data handling from TPS65217 MFD driver to regulator driver.
> > > 
> > > This makes MFD driver independent of REGULATOR framework so
> > > build error is fixed if CONFIG_REGULATOR is not set.
> > > 
> > > drivers/built-in.o: In function `tps65217_probe':
> > > tps65217.c:(.devinit.text+0x13e37): undefined reference
> > > to `of_regulator_match'
> > > 
> > > This patch is based on linux-next (20120720) tree
> > > 
> > > Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> > > ---
> > >  drivers/mfd/tps65217.c                 |  130 +++++++++++---------------------
> > >  drivers/regulator/tps65217-regulator.c |  124 ++++++++++++++++++++++++++----
> > >  include/linux/mfd/tps65217.h           |   12 ++-
> > >  3 files changed, 161 insertions(+), 105 deletions(-)
> > It doesn't apply to my for-next branch. Could you please re-send this one
> > after the merge window is closed ? I'll push that as part of the MFD fixes for
> > 3.6.
> > 
> 
> I fetch regulator tree and applied on top of for-next branch and it is cleanly
> applied.
> 

My bad I picked up wrong tree, I will send the new version (if require) based on
"MFD for-next" tree once the merge window closed.

Thanks,
AnilKumar

RE: [PATCH] mfd/regulator: tps65217: Move regulator plat data handling to regulator

[AnilKumar, Chimata]

+stable

Hi Samuel,

On Wed, Aug 01, 2012 at 10:55:26, AnilKumar, Chimata wrote:
> Hi Samuel,
> 
> On Tue, Jul 31, 2012 at 20:00:21, AnilKumar, Chimata wrote:
> > Hi Samuel,
> > 
> > On Fri, Jul 27, 2012 at 18:18:17, Samuel Ortiz wrote:
> > > Hi Anilkumar,
> > > 
> > > On Fri, Jul 20, 2012 at 03:00:01PM +0530, AnilKumar Ch wrote:
> > > > Regulator platform data handling was mistakenly added to MFD
> > > > driver. So we will see build errors if we compile MFD drivers
> > > > without CONFIG_REGULATOR. This patch moves regulator platform
> > > > data handling from TPS65217 MFD driver to regulator driver.
> > > > 
> > > > This makes MFD driver independent of REGULATOR framework so
> > > > build error is fixed if CONFIG_REGULATOR is not set.
> > > > 
> > > > drivers/built-in.o: In function `tps65217_probe':
> > > > tps65217.c:(.devinit.text+0x13e37): undefined reference
> > > > to `of_regulator_match'
> > > > 
> > > > This patch is based on linux-next (20120720) tree
> > > > 
> > > > Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> > > > ---
> > > >  drivers/mfd/tps65217.c                 |  130 +++++++++++---------------------
> > > >  drivers/regulator/tps65217-regulator.c |  124 ++++++++++++++++++++++++++----
> > > >  include/linux/mfd/tps65217.h           |   12 ++-
> > > >  3 files changed, 161 insertions(+), 105 deletions(-)
> > > It doesn't apply to my for-next branch. Could you please re-send this one
> > > after the merge window is closed ? I'll push that as part of the MFD fixes for
> > > 3.6.
> > > 
> > 
> > I fetch regulator tree and applied on top of for-next branch and it is cleanly
> > applied.
> > 
> 
> My bad I picked up wrong tree, I will send the new version (if require) based on
> "MFD for-next" tree once the merge window closed.
> 

This patch is applied on top of your "MFD master" branch.
Could you please pull this patch ASAP, this patch fixs the
build break.

Thanks
AnilKumar

Re: [PATCH] mfd/regulator: tps65217: Move regulator plat data handling to regulator

[Matthias Kaehlcke]

Hi,

El Fri, Jul 20, 2012 at 03:00:01PM +0530 AnilKumar Ch ha dit:

> Regulator platform data handling was mistakenly added to MFD
> driver. So we will see build errors if we compile MFD drivers
> without CONFIG_REGULATOR. This patch moves regulator platform
> data handling from TPS65217 MFD driver to regulator driver.
> 
>
> diff --git a/drivers/regulator/tps65217-regulator.c b/drivers/regulator/tps65217-regulator.c
> index 6caa222..9730f1d 100644
> --- a/drivers/regulator/tps65217-regulator.c
> +++ b/drivers/regulator/tps65217-regulator.c
>
> +static struct tps65217_board *tps65217_parse_dt(struct platform_device *pdev)
> +{
> +	struct tps65217 *tps = dev_get_drvdata(pdev->dev.parent);
> +	struct device_node *node = tps->dev->of_node;
> +	struct tps65217_board *pdata;
> +	struct device_node *regs;
> +	int i, count;
> +
> +	regs = of_find_node_by_name(node, "regulators");
> +	if (!regs)
> +		return NULL;
> +
> +	count = of_regulator_match(pdev->dev.parent, regs,
> +				reg_matches, TPS65217_NUM_REGULATOR);
> +	of_node_put(regs);
> +	if ((count < 0) || (count > TPS65217_NUM_REGULATOR))
> +		return NULL;
> +
> +	pdata = devm_kzalloc(&pdev->dev, count * sizeof(*pdata),
> GFP_KERNEL);

this allocates a struct tps65217_board for each regulator specified in
the device tree. the structure itself provides arrays for the pointers
to the regulator init data and the regulator device tree node, so only
one instance of it is needed

also the tps65217_board structure should be renamed to something like
tps65217_regulators, now that it is specific to the regulator
driver. besides the regulators the tps65217 can have other subdevices,
so for the non device tree use case it makes sense to keep a chip wide
platform data structure around which contains the subdevice specific
ones

best regards

-- 
Matthias Kaehlcke
Embedded Linux Developer
Amsterdam

                   If you pay peanuts, you get monkeys
                           (James Goldsmith)
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-

RE: [PATCH] mfd/regulator: tps65217: Move regulator plat data handling to regulator

[AnilKumar, Chimata]

Hi Matthias,

On Mon, Aug 13, 2012 at 13:57:32, Matthias Kaehlcke wrote:
> Hi,
> 
> El Fri, Jul 20, 2012 at 03:00:01PM +0530 AnilKumar Ch ha dit:
> 
> > Regulator platform data handling was mistakenly added to MFD
> > driver. So we will see build errors if we compile MFD drivers
> > without CONFIG_REGULATOR. This patch moves regulator platform
> > data handling from TPS65217 MFD driver to regulator driver.
> > 
> >
> > diff --git a/drivers/regulator/tps65217-regulator.c b/drivers/regulator/tps65217-regulator.c
> > index 6caa222..9730f1d 100644
> > --- a/drivers/regulator/tps65217-regulator.c
> > +++ b/drivers/regulator/tps65217-regulator.c
> >
> > +static struct tps65217_board *tps65217_parse_dt(struct platform_device *pdev)
> > +{
> > +	struct tps65217 *tps = dev_get_drvdata(pdev->dev.parent);
> > +	struct device_node *node = tps->dev->of_node;
> > +	struct tps65217_board *pdata;
> > +	struct device_node *regs;
> > +	int i, count;
> > +
> > +	regs = of_find_node_by_name(node, "regulators");
> > +	if (!regs)
> > +		return NULL;
> > +
> > +	count = of_regulator_match(pdev->dev.parent, regs,
> > +				reg_matches, TPS65217_NUM_REGULATOR);
> > +	of_node_put(regs);
> > +	if ((count < 0) || (count > TPS65217_NUM_REGULATOR))
> > +		return NULL;
> > +
> > +	pdata = devm_kzalloc(&pdev->dev, count * sizeof(*pdata),
> > GFP_KERNEL);
> 
> this allocates a struct tps65217_board for each regulator specified in
> the device tree. the structure itself provides arrays for the pointers
> to the regulator init data and the regulator device tree node, so only
> one instance of it is needed

Agree, I will fix that issue in my next version of patch

> 
> also the tps65217_board structure should be renamed to something like
> tps65217_regulators, now that it is specific to the regulator

Not required because tps65217_board might contain platform data for WLED/
Battery charger driver if require. So the struct was named as tps65217_board

Regards
AnilKumar

Re: [PATCH] mfd/regulator: tps65217: Move regulator plat data handling to regulator

[Matthias Kaehlcke]

Hi AnilKumar,

El Mon, Aug 13, 2012 at 01:35:55PM +0000 AnilKumar, Chimata ha dit:

> > also the tps65217_board structure should be renamed to something like
> > tps65217_regulators, now that it is specific to the regulator
> 
> Not required because tps65217_board might contain platform data for WLED/
> Battery charger driver if require. So the struct was named as tps65217_board

the patch moves the allocation of the structure to the regulator
driver, so either it should be a subdevice specific structure or a
chip specific one which is allocated in the mfd driver. otherwise the
regulator driver will allocate memory which is never used when struct
tps65217_board is extended for other subdevices. and the same would
happen in the other subdevice drivers if they choose to allocate their
own struct tps65217_board instead of a sub-structure with just the
fields needed by the specific driver

regards

-- 
Matthias Kaehlcke
Embedded Linux Developer
Amsterdam

    Yo soy como soy y tú eres como eres, construyamos un mundo donde yo
    pueda ser sin dejar de ser yo, donde tú puedas ser sin dejar de ser
    tú, y donde ni yo ni tú obliguemos al otro a ser como yo o como tú
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html