linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND 0/4] mfd/regulator: tps65090: add DT support and suspend/resume cleanups
@ 2013-01-27  8:57 Laxman Dewangan
  2013-01-27  8:57 ` [PATCH RESEND 1/4] mfd: tps65090: add DT support for tps65090 Laxman Dewangan
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Laxman Dewangan @ 2013-01-27  8:57 UTC (permalink / raw)
  To: sameo
  Cc: grant.likely, rob.herring, broonie, linux-kernel, linux-doc,
	devicetree-discuss, Laxman Dewangan

The patch series add DT support on TPS65090 device.

Also remove the suspend/resume implementation as it duplicates with
irq_suspend/irq_resume().

Resending this patch series incase of it is missed.

Laxman Dewangan (4):
  mfd: tps65090: add DT support for tps65090
  regulator: tps65090: add DT support
  mfd: tps65090: Pass irq domain when adding mfd sub devices
  mfd: tps65090: remove suspend/resume callbacks

 .../devicetree/bindings/regulator/tps65090.txt     |  121 ++++++++++++++++++++
 drivers/mfd/tps65090.c                             |   77 ++++++++----
 drivers/regulator/tps65090-regulator.c             |   96 +++++++++++++++-
 include/linux/mfd/tps65090.h                       |    1 +
 4 files changed, 266 insertions(+), 29 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/tps65090.txt


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH RESEND 1/4] mfd: tps65090: add DT support for tps65090
  2013-01-27  8:57 [PATCH RESEND 0/4] mfd/regulator: tps65090: add DT support and suspend/resume cleanups Laxman Dewangan
@ 2013-01-27  8:57 ` Laxman Dewangan
  2013-01-28 15:52   ` Stephen Warren
  2013-01-27  8:57 ` [PATCH RESEND 2/4] regulator: tps65090: add DT support Laxman Dewangan
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Laxman Dewangan @ 2013-01-27  8:57 UTC (permalink / raw)
  To: sameo
  Cc: grant.likely, rob.herring, broonie, linux-kernel, linux-doc,
	devicetree-discuss, Laxman Dewangan

Add device tree support for the TI PMIC TPS65090.
The device can be registered through platform or DT.

Add device tree binding document for this device.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 .../devicetree/bindings/regulator/tps65090.txt     |  121 ++++++++++++++++++++
 drivers/mfd/tps65090.c                             |   52 ++++++++-
 include/linux/mfd/tps65090.h                       |    1 +
 3 files changed, 172 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/tps65090.txt

diff --git a/Documentation/devicetree/bindings/regulator/tps65090.txt b/Documentation/devicetree/bindings/regulator/tps65090.txt
new file mode 100644
index 0000000..e81f47d
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/tps65090.txt
@@ -0,0 +1,121 @@
+TPS65090 regulators
+
+Required properties:
+- compatible: "ti,tps65090"
+- reg: I2C slave address
+- interrupts: the interrupt outputs of the controller
+- regulators: A node that houses a sub-node for each regulator within the
+  device. Each sub-node is identified using the node's name (or the deprecated
+  regulator-compatible property if present), with valid values listed below.
+  The content of each sub-node is defined by the standard binding for
+  regulators; see regulator.txt.
+  dcdc[1-3], fet[1-7] and ldo[1-2] respectively.
+- vsys[1-3]-supply: The input supply for DCDC[1-3] respectively.
+- infet[1-7]-supply: The input supply for FET[1-7] respectively.
+- vsys_l[1-2]-supply: The input supply for LDO[1-2] respectively.
+
+Optional properties:
+- ti,enable-ext-control: This is applicable for DCDC1, DCDC2 and DCDC3.
+  If DCDCs are externally controlled then this property should be there.
+- gpio: This is applicable for DCDC1, DCDC2 and DCDC3. If DCDCs are
+  extrenally controlled and if it is from GPIO then gpio number should
+  be provided. If it is externally controlled and no gpio entry then
+  driver will just configure this rails as external control and will not
+  provide any enable/disable APIs.
+
+Each regulator is defined using the standard binding for regulators.
+
+Example:
+
+	tps65090@48 {
+		compatible = "ti,tps65090";
+		reg = <0x48>;
+		interrupts = <0 88 0x4>;
+
+		vsys1-supply = <&some_reg>;
+		vsys2-supply = <&some_reg>;
+		vsys3-supply = <&some_reg>;
+		infet1-supply = <&some_reg>;
+		infet2-supply = <&some_reg>;
+		infet3-supply = <&some_reg>;
+		infet4-supply = <&some_reg>;
+		infet5-supply = <&some_reg>;
+		infet6-supply = <&some_reg>;
+		infet7-supply = <&some_reg>;
+		vsys_l1-supply = <&some_reg>;
+		vsys_l2-supply = <&some_reg>;
+
+		regulators {
+			dcdc1 {
+				regulator-name = "dcdc1";
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			dcdc2 {
+				regulator-name = "dcdc2";
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			dcdc3 {
+				regulator-name = "dcdc3";
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			fet1 {
+				regulator-name = "fet1";
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			fet2 {
+				regulator-name = "fet2";
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			fet3 {
+				regulator-name = "fet3";
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			fet4 {
+				regulator-name = "fet4";
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			fet5 {
+				regulator-name = "fet5";
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			fet6 {
+				regulator-name = "fet6";
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			fet7 {
+				regulator-name = "fet7";
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo1 {
+				regulator-name = "ldo1";
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo2 {
+				regulator-name = "ldo2";
+				regulator-boot-on;
+				regulator-always-on;
+			};
+		};
+	};
diff --git a/drivers/mfd/tps65090.c b/drivers/mfd/tps65090.c
index 8d12a8e..e4cf030 100644
--- a/drivers/mfd/tps65090.c
+++ b/drivers/mfd/tps65090.c
@@ -25,6 +25,8 @@
 #include <linux/i2c.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/tps65090.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/err.h>
 
 #define NUM_INT_REG 2
@@ -148,6 +150,37 @@ static const struct regmap_config tps65090_regmap_config = {
 	.volatile_reg = is_volatile_reg,
 };
 
+#ifdef CONFIG_OF
+static const struct of_device_id tps65090_of_match[] = {
+	{ .compatible = "ti,tps65090",},
+	{},
+};
+MODULE_DEVICE_TABLE(of, tps65090_of_match);
+
+static struct tps65090_platform_data *
+	of_get_tps65090_platform_data(struct device *dev)
+{
+	struct tps65090_platform_data *pdata;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(dev, "Memory alloc failed for platform data\n");
+		return ERR_PTR(-ENOMEM);
+	}
+	/*
+	 * Nothing to set/parse here as DT parsing of regulators will be
+	 * done in regulator driver.
+	 */
+	return pdata;
+}
+#else
+static struct tps65090_platform_data *
+	of_get_tps65090_platform_data(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
 static int tps65090_i2c_probe(struct i2c_client *client,
 					const struct i2c_device_id *id)
 {
@@ -155,9 +188,22 @@ static int tps65090_i2c_probe(struct i2c_client *client,
 	struct tps65090 *tps65090;
 	int ret;
 
-	if (!pdata) {
+	if (client->dev.of_node) {
+		const struct of_device_id *match;
+
+		match = of_match_device(of_match_ptr(tps65090_of_match),
+				&client->dev);
+		if (!match) {
+			dev_err(&client->dev, "No match device found\n");
+			return -ENODEV;
+		}
+	}
+
+	if (!pdata && client->dev.of_node)
+		pdata = of_get_tps65090_platform_data(&client->dev);
+	if (IS_ERR_OR_NULL(pdata)) {
 		dev_err(&client->dev, "tps65090 requires platform data\n");
-		return -EINVAL;
+		return (pdata) ? PTR_ERR(pdata) : -EINVAL;
 	}
 
 	tps65090 = devm_kzalloc(&client->dev, sizeof(*tps65090), GFP_KERNEL);
@@ -167,6 +213,7 @@ static int tps65090_i2c_probe(struct i2c_client *client,
 	}
 
 	tps65090->dev = &client->dev;
+	tps65090->pdata = pdata;
 	i2c_set_clientdata(client, tps65090);
 
 	tps65090->rmap = devm_regmap_init_i2c(client, &tps65090_regmap_config);
@@ -247,6 +294,7 @@ static struct i2c_driver tps65090_driver = {
 	.driver	= {
 		.name	= "tps65090",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(tps65090_of_match),
 		.pm	= &tps65090_pm_ops,
 	},
 	.probe		= tps65090_i2c_probe,
diff --git a/include/linux/mfd/tps65090.h b/include/linux/mfd/tps65090.h
index 6694cf4..4403c54 100644
--- a/include/linux/mfd/tps65090.h
+++ b/include/linux/mfd/tps65090.h
@@ -67,6 +67,7 @@ struct tps65090 {
 	struct device		*dev;
 	struct regmap		*rmap;
 	struct regmap_irq_chip_data *irq_data;
+	struct tps65090_platform_data *pdata;
 };
 
 /*
-- 
1.7.1.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH RESEND 2/4] regulator: tps65090: add DT support
  2013-01-27  8:57 [PATCH RESEND 0/4] mfd/regulator: tps65090: add DT support and suspend/resume cleanups Laxman Dewangan
  2013-01-27  8:57 ` [PATCH RESEND 1/4] mfd: tps65090: add DT support for tps65090 Laxman Dewangan
@ 2013-01-27  8:57 ` Laxman Dewangan
  2013-01-28 15:58   ` Stephen Warren
  2013-01-27  8:57 ` [PATCH RESEND 3/4] mfd: tps65090: Pass irq domain when adding mfd sub devices Laxman Dewangan
  2013-01-27  8:57 ` [PATCH RESEND 4/4] mfd: tps65090: remove suspend/resume callbacks Laxman Dewangan
  3 siblings, 1 reply; 7+ messages in thread
From: Laxman Dewangan @ 2013-01-27  8:57 UTC (permalink / raw)
  To: sameo
  Cc: grant.likely, rob.herring, broonie, linux-kernel, linux-doc,
	devicetree-discuss, Laxman Dewangan

Add DT support for TI PMIC tps65090 regulator driver. The DT of this
device have node regulator and all regulator's node of this device is
added under this node.

The device tree binding document has the required information for
adding this device on DTS file.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
Resending patch with reviewed by: Mark as it reviewed the patch.
Also Mark B is fine to take this patch to mfd.

 drivers/regulator/tps65090-regulator.c |   96 +++++++++++++++++++++++++++++++-
 1 files changed, 93 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/tps65090-regulator.c b/drivers/regulator/tps65090-regulator.c
index 41c3917..bc45e98 100644
--- a/drivers/regulator/tps65090-regulator.c
+++ b/drivers/regulator/tps65090-regulator.c
@@ -19,11 +19,13 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/gpio.h>
+#include <linux/of_gpio.h>
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
 #include <linux/mfd/tps65090.h>
 
 struct tps65090_regulator {
@@ -138,6 +140,85 @@ static void tps65090_configure_regulator_config(
 	}
 }
 
+#ifdef CONFIG_OF
+static struct of_regulator_match tps65090_matches[] = {
+	{ .name = "dcdc1", },
+	{ .name = "dcdc2", },
+	{ .name = "dcdc3", },
+	{ .name = "fet1",  },
+	{ .name = "fet2",  },
+	{ .name = "fet3",  },
+	{ .name = "fet4",  },
+	{ .name = "fet5",  },
+	{ .name = "fet6",  },
+	{ .name = "fet7",  },
+	{ .name = "ldo1",  },
+	{ .name = "ldo2",  },
+};
+
+static struct tps65090_platform_data *tps65090_parse_dt_reg_data(
+		struct platform_device *pdev,
+		struct of_regulator_match **tps65090_reg_matches)
+{
+	struct tps65090 *tps65090_mfd = dev_get_drvdata(pdev->dev.parent);
+	struct tps65090_platform_data *tps65090_pdata = tps65090_mfd->pdata;
+	struct device_node *np = pdev->dev.parent->of_node;
+	struct device_node *regulators;
+	int idx = 0, ret;
+	struct tps65090_regulator_plat_data *reg_pdata;
+
+	reg_pdata = devm_kzalloc(&pdev->dev, TPS65090_REGULATOR_MAX *
+				sizeof(*reg_pdata), GFP_KERNEL);
+	if (!reg_pdata) {
+		dev_err(&pdev->dev, "Memory alloc for reg_pdata failed\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	regulators = of_find_node_by_name(np, "regulators");
+	if (!regulators) {
+		dev_err(&pdev->dev, "regulator node not found\n");
+		return ERR_PTR(-ENODEV);
+	}
+
+	ret = of_regulator_match(pdev->dev.parent, regulators, tps65090_matches,
+			ARRAY_SIZE(tps65090_matches));
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"Error parsing regulator init data: %d\n", ret);
+		return ERR_PTR(ret);
+	}
+
+	*tps65090_reg_matches = tps65090_matches;
+	for (idx = 0; idx < ARRAY_SIZE(tps65090_matches); idx++) {
+		struct regulator_init_data *ri_data;
+		struct tps65090_regulator_plat_data *rpdata;
+
+		rpdata = &reg_pdata[idx];
+		ri_data = tps65090_matches[idx].init_data;
+		if (!ri_data || !tps65090_matches[idx].of_node)
+			continue;
+
+		rpdata->reg_init_data = ri_data;
+		rpdata->enable_ext_control = of_property_read_bool(
+					tps65090_matches[idx].of_node,
+					"ti,enable-ext-control");
+		if (rpdata->enable_ext_control)
+			rpdata->gpio = of_get_named_gpio(np, "gpio", 0);
+
+		tps65090_pdata->reg_pdata[idx] = rpdata;
+	}
+	return tps65090_pdata;
+}
+#else
+static inline struct tps65090_platform_data *tps65090_parse_dt_reg_data(
+			struct platform_device *pdev,
+			struct of_regulator_match **tps65090_reg_matches)
+{
+	*tps65090_reg_matches = NULL;
+	return NULL;
+}
+#endif
+
 static int tps65090_regulator_probe(struct platform_device *pdev)
 {
 	struct tps65090 *tps65090_mfd = dev_get_drvdata(pdev->dev.parent);
@@ -147,15 +228,20 @@ static int tps65090_regulator_probe(struct platform_device *pdev)
 	struct tps65090_regulator_plat_data *tps_pdata;
 	struct tps65090_regulator *pmic;
 	struct tps65090_platform_data *tps65090_pdata;
+	struct of_regulator_match *tps65090_reg_matches = NULL;
 	int num;
 	int ret;
 
 	dev_dbg(&pdev->dev, "Probing regulator\n");
 
 	tps65090_pdata = dev_get_platdata(pdev->dev.parent);
-	if (!tps65090_pdata) {
+	if (!tps65090_pdata && tps65090_mfd->pdata &&
+			tps65090_mfd->dev->of_node)
+		tps65090_pdata = tps65090_parse_dt_reg_data(pdev,
+					&tps65090_reg_matches);
+	if (IS_ERR_OR_NULL(tps65090_pdata)) {
 		dev_err(&pdev->dev, "Platform data missing\n");
-		return -EINVAL;
+		return (tps65090_pdata) ? PTR_ERR(tps65090_pdata) : -EINVAL;
 	}
 
 	pmic = devm_kzalloc(&pdev->dev, TPS65090_REGULATOR_MAX * sizeof(*pmic),
@@ -192,13 +278,17 @@ static int tps65090_regulator_probe(struct platform_device *pdev)
 			}
 		}
 
-		config.dev = &pdev->dev;
+		config.dev = pdev->dev.parent;
 		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;
+		if (tps65090_reg_matches)
+			config.of_node = tps65090_reg_matches[num].of_node;
+		else
+			config.of_node = NULL;
 
 		rdev = regulator_register(ri->desc, &config);
 		if (IS_ERR(rdev)) {
-- 
1.7.1.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH RESEND 3/4] mfd: tps65090: Pass irq domain when adding mfd sub devices
  2013-01-27  8:57 [PATCH RESEND 0/4] mfd/regulator: tps65090: add DT support and suspend/resume cleanups Laxman Dewangan
  2013-01-27  8:57 ` [PATCH RESEND 1/4] mfd: tps65090: add DT support for tps65090 Laxman Dewangan
  2013-01-27  8:57 ` [PATCH RESEND 2/4] regulator: tps65090: add DT support Laxman Dewangan
@ 2013-01-27  8:57 ` Laxman Dewangan
  2013-01-27  8:57 ` [PATCH RESEND 4/4] mfd: tps65090: remove suspend/resume callbacks Laxman Dewangan
  3 siblings, 0 replies; 7+ messages in thread
From: Laxman Dewangan @ 2013-01-27  8:57 UTC (permalink / raw)
  To: sameo
  Cc: grant.likely, rob.herring, broonie, linux-kernel, linux-doc,
	devicetree-discuss, Laxman Dewangan

When device is get added through DT then irq_base is 0 (zero)
and in this case regmap_irq_chip_get_base() generates warning.
The interrupt of this device get added through irq_domain_add_linear()
when irq_base is 0.

Hence pass the irq domain in place of base_irq when calling
mfd_add_devices().

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

diff --git a/drivers/mfd/tps65090.c b/drivers/mfd/tps65090.c
index e4cf030..3e987be 100644
--- a/drivers/mfd/tps65090.c
+++ b/drivers/mfd/tps65090.c
@@ -236,7 +236,7 @@ static int tps65090_i2c_probe(struct i2c_client *client,
 
 	ret = mfd_add_devices(tps65090->dev, -1, tps65090s,
 		ARRAY_SIZE(tps65090s), NULL,
-		regmap_irq_chip_get_base(tps65090->irq_data), NULL);
+		0, regmap_irq_get_domain(tps65090->irq_data));
 	if (ret) {
 		dev_err(&client->dev, "add mfd devices failed with err: %d\n",
 			ret);
-- 
1.7.1.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH RESEND 4/4] mfd: tps65090: remove suspend/resume callbacks
  2013-01-27  8:57 [PATCH RESEND 0/4] mfd/regulator: tps65090: add DT support and suspend/resume cleanups Laxman Dewangan
                   ` (2 preceding siblings ...)
  2013-01-27  8:57 ` [PATCH RESEND 3/4] mfd: tps65090: Pass irq domain when adding mfd sub devices Laxman Dewangan
@ 2013-01-27  8:57 ` Laxman Dewangan
  3 siblings, 0 replies; 7+ messages in thread
From: Laxman Dewangan @ 2013-01-27  8:57 UTC (permalink / raw)
  To: sameo
  Cc: grant.likely, rob.herring, broonie, linux-kernel, linux-doc,
	devicetree-discuss, Laxman Dewangan

The tps65090 mfd driver implement the suspend/resume callbacks
which just disable and enable irqs in suspend/resume respectively.

This operation is already done in irq suspend and irq_resume and
hence it is not require to implement the same in the driver.

Remove this non-require code.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
This change is based on Mark Brown's comment when reviewing tps80031 mfd driver
as we do not need suspend/resume just for disabling/enabling interrupts.

Resending the patch wth Mark Acks.

 drivers/mfd/tps65090.c |   23 -----------------------
 1 files changed, 0 insertions(+), 23 deletions(-)

diff --git a/drivers/mfd/tps65090.c b/drivers/mfd/tps65090.c
index 3e987be..995e7b3 100644
--- a/drivers/mfd/tps65090.c
+++ b/drivers/mfd/tps65090.c
@@ -262,28 +262,6 @@ static int tps65090_i2c_remove(struct i2c_client *client)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int tps65090_suspend(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	if (client->irq)
-		disable_irq(client->irq);
-	return 0;
-}
-
-static int tps65090_resume(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	if (client->irq)
-		enable_irq(client->irq);
-	return 0;
-}
-#endif
-
-static const struct dev_pm_ops tps65090_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(tps65090_suspend, tps65090_resume)
-};
-
 static const struct i2c_device_id tps65090_id_table[] = {
 	{ "tps65090", 0 },
 	{ },
@@ -295,7 +273,6 @@ static struct i2c_driver tps65090_driver = {
 		.name	= "tps65090",
 		.owner	= THIS_MODULE,
 		.of_match_table = of_match_ptr(tps65090_of_match),
-		.pm	= &tps65090_pm_ops,
 	},
 	.probe		= tps65090_i2c_probe,
 	.remove		= tps65090_i2c_remove,
-- 
1.7.1.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH RESEND 1/4] mfd: tps65090: add DT support for tps65090
  2013-01-27  8:57 ` [PATCH RESEND 1/4] mfd: tps65090: add DT support for tps65090 Laxman Dewangan
@ 2013-01-28 15:52   ` Stephen Warren
  0 siblings, 0 replies; 7+ messages in thread
From: Stephen Warren @ 2013-01-28 15:52 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: sameo, linux-doc, devicetree-discuss, broonie, linux-kernel, rob.herring

On 01/27/2013 01:57 AM, Laxman Dewangan wrote:
> Add device tree support for the TI PMIC TPS65090.
> The device can be registered through platform or DT.
> 
> Add device tree binding document for this device.

> diff --git a/Documentation/devicetree/bindings/regulator/tps65090.txt b/Documentation/devicetree/bindings/regulator/tps65090.txt

> +Required properties:
> +- compatible: "ti,tps65090"
> +- reg: I2C slave address
> +- interrupts: the interrupt outputs of the controller
> +- regulators: A node that houses a sub-node for each regulator within the
> +  device. Each sub-node is identified using the node's name (or the deprecated
> +  regulator-compatible property if present), with valid values listed below.

It may not be worth mentioning deprecated stuff in a new binding doc.

> +  The content of each sub-node is defined by the standard binding for
> +  regulators; see regulator.txt.
> +  dcdc[1-3], fet[1-7] and ldo[1-2] respectively.
> +- vsys[1-3]-supply: The input supply for DCDC[1-3] respectively.
> +- infet[1-7]-supply: The input supply for FET[1-7] respectively.
> +- vsys_l[1-2]-supply: The input supply for LDO[1-2] respectively.

_ in a DT property name is unusual; perhaps use - instead?

> +Optional properties:
> +- ti,enable-ext-control: This is applicable for DCDC1, DCDC2 and DCDC3.
> +  If DCDCs are externally controlled then this property should be there.
> +- gpio: This is applicable for DCDC1, DCDC2 and DCDC3. If DCDCs are
> +  extrenally controlled and if it is from GPIO then gpio number should

s/extrenally/externally/

GPIO should always be capitalized it text.

"gpio" is a rather generic property name. "dcdc-gpios" or
"dcdc-ext-control-gpios" might be better?

> diff --git a/drivers/mfd/tps65090.c b/drivers/mfd/tps65090.c

> @@ -155,9 +188,22 @@ static int tps65090_i2c_probe(struct i2c_client *client,
>  	struct tps65090 *tps65090;
>  	int ret;
>  
> -	if (!pdata) {
> +	if (client->dev.of_node) {
> +		const struct of_device_id *match;
> +
> +		match = of_match_device(of_match_ptr(tps65090_of_match),
> +				&client->dev);
> +		if (!match) {
> +			dev_err(&client->dev, "No match device found\n");
> +			return -ENODEV;
> +		}

Is that useful; "match" doesn't seem to be used anywhere, and this
driver won't be instantiated through DT unless the driver/I2C core found
a matching entry in tps65090_of_match already.

> +	if (!pdata && client->dev.of_node)
> +		pdata = of_get_tps65090_platform_data(&client->dev);
> +	if (IS_ERR_OR_NULL(pdata)) {
>  		dev_err(&client->dev, "tps65090 requires platform data\n");
> -		return -EINVAL;
> +		return (pdata) ? PTR_ERR(pdata) : -EINVAL;
>  	}

Does the driver really /require/ pdata?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RESEND 2/4] regulator: tps65090: add DT support
  2013-01-27  8:57 ` [PATCH RESEND 2/4] regulator: tps65090: add DT support Laxman Dewangan
@ 2013-01-28 15:58   ` Stephen Warren
  0 siblings, 0 replies; 7+ messages in thread
From: Stephen Warren @ 2013-01-28 15:58 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: sameo, linux-doc, devicetree-discuss, broonie, linux-kernel, rob.herring

On 01/27/2013 01:57 AM, Laxman Dewangan wrote:
> Add DT support for TI PMIC tps65090 regulator driver. The DT of this
> device have node regulator and all regulator's node of this device is
> added under this node.
> 
> The device tree binding document has the required information for
> adding this device on DTS file.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

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

> @@ -147,15 +228,20 @@ static int tps65090_regulator_probe(struct platform_device *pdev)
>  	struct tps65090_regulator_plat_data *tps_pdata;
>  	struct tps65090_regulator *pmic;
>  	struct tps65090_platform_data *tps65090_pdata;
> +	struct of_regulator_match *tps65090_reg_matches = NULL;
>  	int num;
>  	int ret;
>  
>  	dev_dbg(&pdev->dev, "Probing regulator\n");
>  
>  	tps65090_pdata = dev_get_platdata(pdev->dev.parent);
> -	if (!tps65090_pdata) {
> +	if (!tps65090_pdata && tps65090_mfd->pdata &&
> +			tps65090_mfd->dev->of_node)
> +		tps65090_pdata = tps65090_parse_dt_reg_data(pdev,
> +					&tps65090_reg_matches);

Why check "&& tps65090_mfd->pdata" here; why not always parse DT if
(!tps65090_pdata && tps65090_mfd->dev->of_node)? In that condition, the
parent MFD driver will always have allocated tps65090_mfd->pdata.


> +	if (IS_ERR_OR_NULL(tps65090_pdata)) {
>  		dev_err(&pdev->dev, "Platform data missing\n");
> -		return -EINVAL;
> +		return (tps65090_pdata) ? PTR_ERR(tps65090_pdata) : -EINVAL;

No need for () there.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-01-28 15:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-27  8:57 [PATCH RESEND 0/4] mfd/regulator: tps65090: add DT support and suspend/resume cleanups Laxman Dewangan
2013-01-27  8:57 ` [PATCH RESEND 1/4] mfd: tps65090: add DT support for tps65090 Laxman Dewangan
2013-01-28 15:52   ` Stephen Warren
2013-01-27  8:57 ` [PATCH RESEND 2/4] regulator: tps65090: add DT support Laxman Dewangan
2013-01-28 15:58   ` Stephen Warren
2013-01-27  8:57 ` [PATCH RESEND 3/4] mfd: tps65090: Pass irq domain when adding mfd sub devices Laxman Dewangan
2013-01-27  8:57 ` [PATCH RESEND 4/4] mfd: tps65090: remove suspend/resume callbacks Laxman Dewangan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).