linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] regulator: s5m8767: Small fixes and device support
@ 2012-12-10 12:49 Amit Daniel Kachhap
  2012-12-10 12:49 ` [PATCH 1/4] regulator: s5m8767: Fix to work when platform registers less regulators Amit Daniel Kachhap
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Amit Daniel Kachhap @ 2012-12-10 12:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: Sangbeom Kim, Liam Girdwood, Mark Brown, amit.kachhap,
	Thomas Abraham, kgene.kim, linux-samsung-soc

These patch series contains 3 small fixes and device tree support for pmic 
component of s5m8767 regulator driver.

Amit Daniel Kachhap (4):
  regulator: s5m8767: Fix to work when platform registers less
    regulators
  regulator: s5m8767: Fix to read the first DVS register.
  regulator: s5m8767: Fix to work even if no DVS gpio present
  regulator: add device tree support for s5m8767

 .../bindings/regulator/s5m8767-regulator.txt       |  133 +++++++++++
 drivers/mfd/sec-core.c                             |   75 ++++++-
 drivers/regulator/s5m8767.c                        |  230 ++++++++++++++++++--
 include/linux/mfd/samsung/core.h                   |    3 +
 4 files changed, 424 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt


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

* [PATCH 1/4] regulator: s5m8767: Fix to work when platform registers less regulators
  2012-12-10 12:49 [PATCH 0/4] regulator: s5m8767: Small fixes and device support Amit Daniel Kachhap
@ 2012-12-10 12:49 ` Amit Daniel Kachhap
  2012-12-11  3:44   ` Mark Brown
  2012-12-10 12:49 ` [PATCH 2/4] regulator: s5m8767: Fix to read the first DVS register Amit Daniel Kachhap
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Amit Daniel Kachhap @ 2012-12-10 12:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: Sangbeom Kim, Liam Girdwood, Mark Brown, amit.kachhap,
	Thomas Abraham, kgene.kim, linux-samsung-soc

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/regulator/s5m8767.c |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
index 2b822be..df0b094 100644
--- a/drivers/regulator/s5m8767.c
+++ b/drivers/regulator/s5m8767.c
@@ -168,7 +168,7 @@ static unsigned int s5m8767_opmode_reg[][4] = {
 static int s5m8767_get_register(struct regulator_dev *rdev, int *reg,
 				int *enable_ctrl)
 {
-	int reg_id = rdev_get_id(rdev);
+	int i, reg_id = rdev_get_id(rdev);
 	unsigned int mode;
 	struct s5m8767_info *s5m8767 = rdev_get_drvdata(rdev);
 
@@ -195,8 +195,17 @@ static int s5m8767_get_register(struct regulator_dev *rdev, int *reg,
 		return -EINVAL;
 	}
 
-	mode = s5m8767->opmode[reg_id].mode;
-	*enable_ctrl = s5m8767_opmode_reg[reg_id][mode] << S5M8767_ENCTRL_SHIFT;
+	for (i = 0; i < s5m8767->num_regulators; i++) {
+		if (s5m8767->opmode[i].id == reg_id) {
+			mode = s5m8767->opmode[i].mode;
+			break;
+		}
+	}
+
+	if (i < s5m8767->num_regulators)
+		*enable_ctrl =
+		s5m8767_opmode_reg[reg_id][mode] << S5M8767_ENCTRL_SHIFT;
+
 	return 0;
 }
 
@@ -547,7 +556,7 @@ static int s5m8767_pmic_probe(struct platform_device *pdev)
 	rdev = s5m8767->rdev;
 	s5m8767->dev = &pdev->dev;
 	s5m8767->iodev = iodev;
-	s5m8767->num_regulators = S5M8767_REG_MAX - 2;
+	s5m8767->num_regulators = pdata->num_regulators;
 	platform_set_drvdata(pdev, s5m8767);
 
 	s5m8767->buck_gpioindex = pdata->buck_default_idx;
-- 
1.7.1


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

* [PATCH 2/4] regulator: s5m8767: Fix to read the first DVS register.
  2012-12-10 12:49 [PATCH 0/4] regulator: s5m8767: Small fixes and device support Amit Daniel Kachhap
  2012-12-10 12:49 ` [PATCH 1/4] regulator: s5m8767: Fix to work when platform registers less regulators Amit Daniel Kachhap
@ 2012-12-10 12:49 ` Amit Daniel Kachhap
  2012-12-11  3:47   ` Mark Brown
  2012-12-10 12:49 ` [PATCH 3/4] regulator: s5m8767: Fix to work even if no DVS gpio present Amit Daniel Kachhap
  2012-12-10 12:49 ` [PATCH 4/4] regulator: add device tree support for s5m8767 Amit Daniel Kachhap
  3 siblings, 1 reply; 9+ messages in thread
From: Amit Daniel Kachhap @ 2012-12-10 12:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: Sangbeom Kim, Liam Girdwood, Mark Brown, amit.kachhap,
	Thomas Abraham, kgene.kim, linux-samsung-soc

This patch modifies the DVS register read function to select correct DVS1
register. This change is required because the GPIO select pin is 000 in
unintialized state and hence selects the DVS1 register.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/regulator/s5m8767.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
index df0b094..7ed7591 100644
--- a/drivers/regulator/s5m8767.c
+++ b/drivers/regulator/s5m8767.c
@@ -272,17 +272,17 @@ static int s5m8767_get_voltage_register(struct regulator_dev *rdev, int *_reg)
 		reg = S5M8767_REG_BUCK1CTRL2;
 		break;
 	case S5M8767_BUCK2:
-		reg = S5M8767_REG_BUCK2DVS2;
+		reg = S5M8767_REG_BUCK2DVS1;
 		if (s5m8767->buck2_gpiodvs)
 			reg += s5m8767->buck_gpioindex;
 		break;
 	case S5M8767_BUCK3:
-		reg = S5M8767_REG_BUCK3DVS2;
+		reg = S5M8767_REG_BUCK3DVS1;
 		if (s5m8767->buck3_gpiodvs)
 			reg += s5m8767->buck_gpioindex;
 		break;
 	case S5M8767_BUCK4:
-		reg = S5M8767_REG_BUCK4DVS2;
+		reg = S5M8767_REG_BUCK4DVS1;
 		if (s5m8767->buck4_gpiodvs)
 			reg += s5m8767->buck_gpioindex;
 		break;
-- 
1.7.1


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

* [PATCH 3/4] regulator: s5m8767: Fix to work even if no DVS gpio present
  2012-12-10 12:49 [PATCH 0/4] regulator: s5m8767: Small fixes and device support Amit Daniel Kachhap
  2012-12-10 12:49 ` [PATCH 1/4] regulator: s5m8767: Fix to work when platform registers less regulators Amit Daniel Kachhap
  2012-12-10 12:49 ` [PATCH 2/4] regulator: s5m8767: Fix to read the first DVS register Amit Daniel Kachhap
@ 2012-12-10 12:49 ` Amit Daniel Kachhap
  2012-12-11  3:48   ` Mark Brown
  2012-12-10 12:49 ` [PATCH 4/4] regulator: add device tree support for s5m8767 Amit Daniel Kachhap
  3 siblings, 1 reply; 9+ messages in thread
From: Amit Daniel Kachhap @ 2012-12-10 12:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: Sangbeom Kim, Liam Girdwood, Mark Brown, amit.kachhap,
	Thomas Abraham, kgene.kim, linux-samsung-soc

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/regulator/s5m8767.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
index 7ed7591..9f991f2 100644
--- a/drivers/regulator/s5m8767.c
+++ b/drivers/regulator/s5m8767.c
@@ -626,9 +626,16 @@ static int s5m8767_pmic_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (gpio_is_valid(pdata->buck_gpios[0]) &&
-		gpio_is_valid(pdata->buck_gpios[1]) &&
-		gpio_is_valid(pdata->buck_gpios[2])) {
+	if (pdata->buck2_gpiodvs || pdata->buck3_gpiodvs ||
+						pdata->buck4_gpiodvs) {
+
+		if (!gpio_is_valid(pdata->buck_gpios[0]) ||
+			!gpio_is_valid(pdata->buck_gpios[1]) ||
+			!gpio_is_valid(pdata->buck_gpios[2])) {
+			dev_err(&pdev->dev, "GPIO NOT VALID\n");
+			return -EINVAL;
+		}
+
 		ret = devm_gpio_request(&pdev->dev, pdata->buck_gpios[0],
 					"S5M8767 SET1");
 		if (ret)
@@ -653,10 +660,6 @@ static int s5m8767_pmic_probe(struct platform_device *pdev)
 		/* SET3 GPIO */
 		gpio_direction_output(pdata->buck_gpios[2],
 				(s5m8767->buck_gpioindex >> 0) & 0x1);
-	} else {
-		dev_err(&pdev->dev, "GPIO NOT VALID\n");
-		ret = -EINVAL;
-		return ret;
 	}
 
 	ret = devm_gpio_request(&pdev->dev, pdata->buck_ds[0], "S5M8767 DS2");
-- 
1.7.1


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

* [PATCH 4/4] regulator: add device tree support for s5m8767
  2012-12-10 12:49 [PATCH 0/4] regulator: s5m8767: Small fixes and device support Amit Daniel Kachhap
                   ` (2 preceding siblings ...)
  2012-12-10 12:49 ` [PATCH 3/4] regulator: s5m8767: Fix to work even if no DVS gpio present Amit Daniel Kachhap
@ 2012-12-10 12:49 ` Amit Daniel Kachhap
  2013-01-08 22:07   ` Thomas Abraham
  3 siblings, 1 reply; 9+ messages in thread
From: Amit Daniel Kachhap @ 2012-12-10 12:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: Sangbeom Kim, Liam Girdwood, Mark Brown, amit.kachhap,
	Thomas Abraham, kgene.kim, linux-samsung-soc

Add device tree based discovery support for pmic block of s5m8767

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 .../bindings/regulator/s5m8767-regulator.txt       |  133 ++++++++++++++
 drivers/mfd/sec-core.c                             |   75 ++++++++-
 drivers/regulator/s5m8767.c                        |  190 +++++++++++++++++++-
 include/linux/mfd/samsung/core.h                   |    3 +
 4 files changed, 398 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt

diff --git a/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt b/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
new file mode 100644
index 0000000..59c372b
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
@@ -0,0 +1,133 @@
+* Samsung S5M8767 Voltage and Current Regulator
+
+The Samsung S5M8767 is a multi-function device which includes volatage and
+current regulators, rtc, charger controller and other sub-blocks. It is
+interfaced to the host controller using a i2c interface. Each sub-block is
+addressed by the host system using different i2c slave address. This document
+describes the bindings for 'pmic' sub-block of s5m8767.
+
+Required properties:
+- compatible: Should be "samsung,s5m8767-pmic".
+- reg: Specifies the i2c slave address of the pmic block. It should be 0x66.
+
+- s5m8767,pmic-buck2-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
+  units for buck2 when changing voltage using gpio dvs. Refer to [1] below
+  for additional information.
+
+- s5m8767,pmic-buck3-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
+  units for buck3 when changing voltage using gpio dvs. Refer to [1] below
+  for additional information.
+
+- s5m8767,pmic-buck4-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
+  units for buck4 when changing voltage using gpio dvs. Refer to [1] below
+  for additional information.
+
+[1] If none of the 's5m8767,pmic-buck[2/3/4]-uses-gpio-dvs' optional
+    property is specified, the 's5m8767,pmic-buck[2/3/4]-dvs-voltage'
+    property should specify atleast one voltage level (which would be a
+    safe operating voltage).
+
+    If either of the 's5m8767,pmic-buck[2/3/4]-uses-gpio-dvs' optional
+    property is specified, then all the eigth voltage values for the
+    's5m8767,pmic-buck[2/3/4]-dvs-voltage' should be specified.
+
+Optional properties:
+- interrupt-parent: Specifies the phandle of the interrupt controller to which
+  the interrupts from s5m8767 are delivered to.
+- interrupts: Interrupt specifiers for two interrupt sources.
+  - First interrupt specifier is for 'irq1' interrupt.
+  - Second interrupt specifier is for 'alert' interrupt.
+- s5m8767,pmic-buck2-uses-gpio-dvs: 'buck2' can be controlled by gpio dvs.
+- s5m8767,pmic-buck3-uses-gpio-dvs: 'buck3' can be controlled by gpio dvs.
+- s5m8767,pmic-buck4-uses-gpio-dvs: 'buck4' can be controlled by gpio dvs.
+
+Additional properties required if either of the optional properties are used:
+
+- s5m8767,pmic-buck234-default-dvs-idx: Default voltage setting selected from
+  the possible 8 options selectable by the dvs gpios. The value of this
+  property should be between 0 and 7. If not specified or if out of range, the
+  default value of this property is set to 0.
+
+- s5m8767,pmic-buck-dvs-gpios: GPIO specifiers for three host gpio's used
+  for dvs. The format of the gpio specifier depends in the gpio controller.
+
+Regulators: The regulators of s5m8767 that have to be instantiated should be
+included in a sub-node named 'regulators'. Regulator nodes included in this
+sub-node should be of the format as listed below.
+
+	regulator_name {
+		standard regulator bindings here
+	};
+
+The following are the names of the regulators that the s5m8767 pmic block
+supports. Note: The 'n' in LDOn and BUCKn represents the LDO or BUCK number
+as per the datasheet of s5m8767.
+
+	- LDOn
+		  - valid values for n are 1 to 28
+		  - Example: LDO0, LD01, LDO28
+	- BUCKn
+		  - valid values for n are 1 to 9.
+		  - Example: BUCK1, BUCK2, BUCK9
+
+The bindings inside the regulator nodes use the standard regulator bindings
+which are documented elsewhere.
+
+Example:
+
+	s5m8767_pmic@66 {
+		compatible = "samsung,s5m8767-pmic";
+		reg = <0x66>;
+
+		s5m8767,pmic-buck2-uses-gpio-dvs;
+		s5m8767,pmic-buck3-uses-gpio-dvs;
+		s5m8767,pmic-buck4-uses-gpio-dvs;
+
+		s5m8767,pmic-buck-default-dvs-idx = <0>;
+
+		s5m8767,pmic-buck-dvs-gpios = <&gpx0 0 1 0 0>, /* DVS1 */
+						 <&gpx0 1 1 0 0>, /* DVS2 */
+						 <&gpx0 2 1 0 0>; /* DVS3 */
+
+		s5m8767,pmic-buck-ds-gpios = <&gpx2 3 1 0 0>, /* SET1 */
+						<&gpx2 4 1 0 0>, /* SET2 */
+						<&gpx2 5 1 0 0>; /* SET3 */
+
+		s5m8767,pmic-buck2-dvs-voltage = <1350000>, <1300000>,
+						 <1250000>, <1200000>,
+						 <1150000>, <1100000>,
+						 <1000000>, <950000>;
+
+		s5m8767,pmic-buck3-dvs-voltage = <1100000>, <1100000>,
+						 <1100000>, <1100000>,
+						 <1000000>, <1000000>,
+						 <1000000>, <1000000>;
+
+		s5m8767,pmic-buck4-dvs-voltage = <1200000>, <1200000>,
+						 <1200000>, <1200000>,
+						 <1200000>, <1200000>,
+						 <1200000>, <1200000>;
+
+		regulators {
+			ldo1_reg: LDO1 {
+				regulator-name = "VDD_ABB_3.3V";
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+			};
+
+			ldo2_reg: LDO2 {
+				regulator-name = "VDD_ALIVE_1.1V";
+				regulator-min-microvolt = <1100000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-always-on;
+			};
+
+			buck1_reg: BUCK1 {
+				regulator-name = "VDD_MIF_1.2V";
+				regulator-min-microvolt = <950000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+		};
+	};
diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
index 49d361a..51de531 100644
--- a/drivers/mfd/sec-core.c
+++ b/drivers/mfd/sec-core.c
@@ -17,6 +17,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/i2c.h>
+#include <linux/of_irq.h>
 #include <linux/interrupt.h>
 #include <linux/pm_runtime.h>
 #include <linux/mutex.h>
@@ -60,6 +61,15 @@ static struct mfd_cell s2mps11_devs[] = {
 	},
 };
 
+#ifdef CONFIG_OF
+static struct of_device_id __devinitdata sec_dt_match[] = {
+	{	.compatible = "samsung,s5m8767-pmic",
+		.data = S5M8767X,
+	},
+	{},
+};
+#endif
+
 int sec_reg_read(struct sec_pmic_dev *sec_pmic, u8 reg, void *dest)
 {
 	return regmap_read(sec_pmic->regmap, reg, dest);
@@ -95,6 +105,57 @@ static struct regmap_config sec_regmap_config = {
 	.val_bits = 8,
 };
 
+
+#ifdef CONFIG_OF
+/*
+ * Only the common platform data elements for s5m8767 are parsed here from the
+ * device tree. Other sub-modules of s5m8767 such as pmic, rtc , charger and
+ * others have to parse their own platform data elements from device tree.
+ *
+ * The s5m8767 platform data structure is instantiated here and the drivers for
+ * the sub-modules need not instantiate another instance while parsing their
+ * platform data.
+ */
+static struct sec_platform_data *sec_pmic_i2c_parse_dt_pdata(
+					struct device *dev)
+{
+	struct sec_platform_data *pd;
+
+	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd) {
+		dev_err(dev, "could not allocate memory for pdata\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	/*
+	 * ToDo: the 'wakeup' member in the platform data is more of a linux
+	 * specfic information. Hence, there is no binding for that yet and
+	 * not parsed here.
+	 */
+
+	return pd;
+}
+#else
+static struct sec_platform_data *sec_i2c_parse_dt_pdata(
+					struct device *dev)
+{
+	return 0;
+}
+#endif
+
+static inline int sec_i2c_get_driver_data(struct i2c_client *i2c,
+						const struct i2c_device_id *id)
+{
+#ifdef CONFIG_OF
+	if (i2c->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_node(sec_dt_match, i2c->dev.of_node);
+		return (int)match->data;
+	}
+#endif
+	return (int)id->driver_data;
+}
+
 static int sec_pmic_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
@@ -111,13 +172,22 @@ static int sec_pmic_probe(struct i2c_client *i2c,
 	sec_pmic->dev = &i2c->dev;
 	sec_pmic->i2c = i2c;
 	sec_pmic->irq = i2c->irq;
-	sec_pmic->type = id->driver_data;
-
+	sec_pmic->type = sec_i2c_get_driver_data(i2c, id);
+
+	if (sec_pmic->dev->of_node) {
+		pdata = sec_pmic_i2c_parse_dt_pdata(sec_pmic->dev);
+		if (IS_ERR(pdata)) {
+			ret = PTR_ERR(pdata);
+			return ret;
+		}
+		pdata->device_type = sec_pmic->type;
+	}
 	if (pdata) {
 		sec_pmic->device_type = pdata->device_type;
 		sec_pmic->ono = pdata->ono;
 		sec_pmic->irq_base = pdata->irq_base;
 		sec_pmic->wakeup = pdata->wakeup;
+		sec_pmic->pdata = pdata;
 	}
 
 	sec_pmic->regmap = devm_regmap_init_i2c(i2c, &sec_regmap_config);
@@ -192,6 +262,7 @@ static struct i2c_driver sec_pmic_driver = {
 	.driver = {
 		   .name = "sec_pmic",
 		   .owner = THIS_MODULE,
+		   .of_match_table = of_match_ptr(sec_dt_match),
 	},
 	.probe = sec_pmic_probe,
 	.remove = sec_pmic_remove,
diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
index 9f991f2..fcd606d 100644
--- a/drivers/regulator/s5m8767.c
+++ b/drivers/regulator/s5m8767.c
@@ -14,6 +14,7 @@
 #include <linux/bug.h>
 #include <linux/err.h>
 #include <linux/gpio.h>
+#include <linux/of_gpio.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -21,6 +22,11 @@
 #include <linux/regulator/machine.h>
 #include <linux/mfd/samsung/core.h>
 #include <linux/mfd/samsung/s5m8767.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/io.h>
+#include <mach/map.h>
+
+#define S5M8767_OPMODE_NORMAL_MODE 0x1
 
 struct s5m8767_info {
 	struct device *dev;
@@ -508,15 +514,196 @@ static struct regulator_desc regulators[] = {
 	s5m8767_regulator_desc(BUCK9),
 };
 
+#ifdef CONFIG_OF
+static int s5m8767_pmic_dt_parse_dvs_gpio(struct sec_pmic_dev *iodev,
+			struct sec_platform_data *pdata,
+			struct device_node *pmic_np)
+{
+	int i, gpio;
+
+	for (i = 0; i < 3; i++) {
+		gpio = of_get_named_gpio(pmic_np,
+					"s5m8767,pmic-buck-dvs-gpios", i);
+		if (!gpio_is_valid(gpio)) {
+			dev_err(iodev->dev, "invalid gpio[%d]: %d\n", i, gpio);
+			return -EINVAL;
+		}
+		pdata->buck_gpios[i] = gpio;
+	}
+	return 0;
+}
+
+static int s5m8767_pmic_dt_parse_ds_gpio(struct sec_pmic_dev *iodev,
+			struct sec_platform_data *pdata,
+			struct device_node *pmic_np)
+{
+	int i, gpio;
+
+	for (i = 0; i < 3; i++) {
+		gpio = of_get_named_gpio(pmic_np,
+					"s5m8767,pmic-buck-ds-gpios", i);
+		if (!gpio_is_valid(gpio)) {
+			dev_err(iodev->dev, "invalid gpio[%d]: %d\n", i, gpio);
+			return -EINVAL;
+		}
+		pdata->buck_ds[i] = gpio;
+	}
+	return 0;
+}
+
+static int s5m8767_pmic_dt_parse_pdata(struct sec_pmic_dev *iodev,
+					struct sec_platform_data *pdata)
+{
+	struct device_node *pmic_np, *regulators_np, *reg_np;
+	struct sec_regulator_data *rdata;
+	struct sec_opmode_data *rmode;
+	unsigned int i, dvs_voltage_nr = 1, ret;
+
+	pmic_np = iodev->dev->of_node;
+	if (!pmic_np) {
+		dev_err(iodev->dev, "could not find pmic sub-node\n");
+		return -ENODEV;
+	}
+
+	regulators_np = of_find_node_by_name(pmic_np, "regulators");
+	if (!regulators_np) {
+		dev_err(iodev->dev, "could not find regulators sub-node\n");
+		return -EINVAL;
+	}
+
+	/* count the number of regulators to be supported in pmic */
+	pdata->num_regulators = 0;
+	for_each_child_of_node(regulators_np, reg_np) {
+		pdata->num_regulators++;
+	}
+
+	rdata = devm_kzalloc(iodev->dev, sizeof(*rdata) *
+				pdata->num_regulators, GFP_KERNEL);
+	if (!rdata) {
+		dev_err(iodev->dev,
+			"could not allocate memory for regulator data\n");
+		return -ENOMEM;
+	}
+
+	rmode = devm_kzalloc(iodev->dev, sizeof(*rmode) *
+				pdata->num_regulators, GFP_KERNEL);
+	if (!rdata) {
+		dev_err(iodev->dev,
+			"could not allocate memory for regulator mode\n");
+		return -ENOMEM;
+	}
+
+
+	pdata->regulators = rdata;
+	pdata->opmode = rmode;
+	for_each_child_of_node(regulators_np, reg_np) {
+		for (i = 0; i < ARRAY_SIZE(regulators); i++)
+			if (!of_node_cmp(reg_np->name, regulators[i].name))
+				break;
+
+		if (i == ARRAY_SIZE(regulators)) {
+			dev_warn(iodev->dev,
+			"don't know how to configure regulator %s\n",
+			reg_np->name);
+			continue;
+		}
+
+		rdata->id = i;
+		rdata->initdata = of_get_regulator_init_data(
+						iodev->dev, reg_np);
+		rdata->reg_node = reg_np;
+		rdata++;
+		rmode->id = i;
+		if (of_property_read_u32(reg_np, "op_mode",
+				&rmode->mode)) {
+			dev_warn(iodev->dev, "no op_mode property property at %s\n",
+				reg_np->full_name);
+
+			rmode->mode = S5M8767_OPMODE_NORMAL_MODE;
+		}
+		rmode++;
+	}
+
+	if (of_get_property(pmic_np, "s5m8767,pmic-buck2-uses-gpio-dvs", NULL))
+		pdata->buck2_gpiodvs = true;
+
+	if (of_get_property(pmic_np, "s5m8767,pmic-buck3-uses-gpio-dvs", NULL))
+		pdata->buck3_gpiodvs = true;
+
+	if (of_get_property(pmic_np, "s5m8767,pmic-buck4-uses-gpio-dvs", NULL))
+		pdata->buck4_gpiodvs = true;
+
+	if (pdata->buck2_gpiodvs || pdata->buck3_gpiodvs ||
+						pdata->buck4_gpiodvs) {
+		ret = s5m8767_pmic_dt_parse_dvs_gpio(iodev, pdata, pmic_np);
+		if (ret)
+			return -EINVAL;
+
+		if (of_property_read_u32(pmic_np,
+				"s5m8767,pmic-buck-default-dvs-idx",
+				&pdata->buck_default_idx)) {
+			pdata->buck_default_idx = 0;
+		} else {
+			if (pdata->buck_default_idx >= 8) {
+				pdata->buck_default_idx = 0;
+				dev_info(iodev->dev,
+				"invalid value for default dvs index, use 0\n");
+			}
+		}
+
+		dvs_voltage_nr = 8;
+	}
+
+	ret = s5m8767_pmic_dt_parse_ds_gpio(iodev, pdata, pmic_np);
+	if (ret)
+		return -EINVAL;
+
+	if (of_property_read_u32_array(pmic_np,
+				"s5m8767,pmic-buck2-dvs-voltage",
+				pdata->buck2_voltage, dvs_voltage_nr)) {
+		dev_err(iodev->dev, "buck2 voltages not specified\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32_array(pmic_np,
+				"s5m8767,pmic-buck3-dvs-voltage",
+				pdata->buck3_voltage, dvs_voltage_nr)) {
+		dev_err(iodev->dev, "buck3 voltages not specified\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32_array(pmic_np,
+				"s5m8767,pmic-buck4-dvs-voltage",
+				pdata->buck4_voltage, dvs_voltage_nr)) {
+		dev_err(iodev->dev, "buck4 voltages not specified\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+#else
+static int s5m8767_pmic_dt_parse_pdata(struct sec_pmic_dev *iodev,
+					struct sec_platform_data *pdata)
+{
+	return 0;
+}
+#endif /* CONFIG_OF */
+
 static int s5m8767_pmic_probe(struct platform_device *pdev)
 {
 	struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
-	struct sec_platform_data *pdata = dev_get_platdata(iodev->dev);
+	struct sec_platform_data *pdata = iodev->pdata;
 	struct regulator_config config = { };
 	struct regulator_dev **rdev;
 	struct s5m8767_info *s5m8767;
 	int i, ret, size, buck_init;
 
+	if (iodev->dev->of_node) {
+		ret = s5m8767_pmic_dt_parse_pdata(iodev, pdata);
+		if (ret)
+			return ret;
+	}
+
 	if (!pdata) {
 		dev_err(pdev->dev.parent, "Platform data not supplied\n");
 		return -ENODEV;
@@ -765,6 +952,7 @@ static int s5m8767_pmic_probe(struct platform_device *pdev)
 		config.dev = s5m8767->dev;
 		config.init_data = pdata->regulators[i].initdata;
 		config.driver_data = s5m8767;
+		config.of_node = pdata->regulators[i].reg_node;
 
 		rdev[i] = regulator_register(&regulators[id], &config);
 		if (IS_ERR(rdev[i])) {
diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
index b50c38f..380ed95 100644
--- a/include/linux/mfd/samsung/core.h
+++ b/include/linux/mfd/samsung/core.h
@@ -26,6 +26,7 @@ enum sec_device_type {
 /**
  * struct sec_pmic_dev - s5m87xx master device for sub-drivers
  * @dev: master device of the chip (can be used to access platform data)
+ * @pdata: pointer to private data used to pass platform data to child
  * @i2c: i2c client private data for regulator
  * @rtc: i2c client private data for rtc
  * @iolock: mutex for serializing io access
@@ -39,6 +40,7 @@ enum sec_device_type {
  */
 struct sec_pmic_dev {
 	struct device *dev;
+	struct sec_platform_data *pdata;
 	struct regmap *regmap;
 	struct i2c_client *i2c;
 	struct i2c_client *rtc;
@@ -127,6 +129,7 @@ struct sec_platform_data {
 struct sec_regulator_data {
 	int				id;
 	struct regulator_init_data	*initdata;
+	struct device_node *reg_node;
 };
 
 /*
-- 
1.7.1


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

* Re: [PATCH 1/4] regulator: s5m8767: Fix to work when platform registers less regulators
  2012-12-10 12:49 ` [PATCH 1/4] regulator: s5m8767: Fix to work when platform registers less regulators Amit Daniel Kachhap
@ 2012-12-11  3:44   ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2012-12-11  3:44 UTC (permalink / raw)
  To: Amit Daniel Kachhap
  Cc: linux-kernel, Sangbeom Kim, Liam Girdwood, amit.kachhap,
	Thomas Abraham, kgene.kim, linux-samsung-soc

On Mon, Dec 10, 2012 at 06:19:39PM +0530, Amit Daniel Kachhap wrote:
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>

Applied, thanks.

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

* Re: [PATCH 2/4] regulator: s5m8767: Fix to read the first DVS register.
  2012-12-10 12:49 ` [PATCH 2/4] regulator: s5m8767: Fix to read the first DVS register Amit Daniel Kachhap
@ 2012-12-11  3:47   ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2012-12-11  3:47 UTC (permalink / raw)
  To: Amit Daniel Kachhap
  Cc: linux-kernel, Sangbeom Kim, Liam Girdwood, amit.kachhap,
	Thomas Abraham, kgene.kim, linux-samsung-soc

On Mon, Dec 10, 2012 at 06:19:40PM +0530, Amit Daniel Kachhap wrote:
> This patch modifies the DVS register read function to select correct DVS1
> register. This change is required because the GPIO select pin is 000 in
> unintialized state and hence selects the DVS1 register.

Applied, thanks.

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

* Re: [PATCH 3/4] regulator: s5m8767: Fix to work even if no DVS gpio present
  2012-12-10 12:49 ` [PATCH 3/4] regulator: s5m8767: Fix to work even if no DVS gpio present Amit Daniel Kachhap
@ 2012-12-11  3:48   ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2012-12-11  3:48 UTC (permalink / raw)
  To: Amit Daniel Kachhap
  Cc: linux-kernel, Sangbeom Kim, Liam Girdwood, amit.kachhap,
	Thomas Abraham, kgene.kim, linux-samsung-soc

On Mon, Dec 10, 2012 at 06:19:41PM +0530, Amit Daniel Kachhap wrote:
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>

Applied, thanks.

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

* Re: [PATCH 4/4] regulator: add device tree support for s5m8767
  2012-12-10 12:49 ` [PATCH 4/4] regulator: add device tree support for s5m8767 Amit Daniel Kachhap
@ 2013-01-08 22:07   ` Thomas Abraham
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Abraham @ 2013-01-08 22:07 UTC (permalink / raw)
  To: Amit Daniel Kachhap
  Cc: linux-kernel, Sangbeom Kim, Liam Girdwood, Mark Brown,
	amit.kachhap, kgene.kim, linux-samsung-soc

On 10 December 2012 04:49, Amit Daniel Kachhap <amit.daniel@samsung.com> wrote:
> Add device tree based discovery support for pmic block of s5m8767
>
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  .../bindings/regulator/s5m8767-regulator.txt       |  133 ++++++++++++++
>  drivers/mfd/sec-core.c                             |   75 ++++++++-
>  drivers/regulator/s5m8767.c                        |  190 +++++++++++++++++++-
>  include/linux/mfd/samsung/core.h                   |    3 +
>  4 files changed, 398 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
>
> diff --git a/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt b/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
> new file mode 100644
> index 0000000..59c372b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
> @@ -0,0 +1,133 @@
> +* Samsung S5M8767 Voltage and Current Regulator
> +
> +The Samsung S5M8767 is a multi-function device which includes volatage and
> +current regulators, rtc, charger controller and other sub-blocks. It is
> +interfaced to the host controller using a i2c interface. Each sub-block is
> +addressed by the host system using different i2c slave address. This document
> +describes the bindings for 'pmic' sub-block of s5m8767.
> +
> +Required properties:
> +- compatible: Should be "samsung,s5m8767-pmic".
> +- reg: Specifies the i2c slave address of the pmic block. It should be 0x66.
> +
> +- s5m8767,pmic-buck2-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
> +  units for buck2 when changing voltage using gpio dvs. Refer to [1] below
> +  for additional information.
> +
> +- s5m8767,pmic-buck3-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
> +  units for buck3 when changing voltage using gpio dvs. Refer to [1] below
> +  for additional information.
> +
> +- s5m8767,pmic-buck4-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
> +  units for buck4 when changing voltage using gpio dvs. Refer to [1] below
> +  for additional information.
> +
> +[1] If none of the 's5m8767,pmic-buck[2/3/4]-uses-gpio-dvs' optional
> +    property is specified, the 's5m8767,pmic-buck[2/3/4]-dvs-voltage'
> +    property should specify atleast one voltage level (which would be a
> +    safe operating voltage).
> +
> +    If either of the 's5m8767,pmic-buck[2/3/4]-uses-gpio-dvs' optional
> +    property is specified, then all the eigth voltage values for the
> +    's5m8767,pmic-buck[2/3/4]-dvs-voltage' should be specified.
> +
> +Optional properties:
> +- interrupt-parent: Specifies the phandle of the interrupt controller to which
> +  the interrupts from s5m8767 are delivered to.
> +- interrupts: Interrupt specifiers for two interrupt sources.
> +  - First interrupt specifier is for 'irq1' interrupt.
> +  - Second interrupt specifier is for 'alert' interrupt.
> +- s5m8767,pmic-buck2-uses-gpio-dvs: 'buck2' can be controlled by gpio dvs.
> +- s5m8767,pmic-buck3-uses-gpio-dvs: 'buck3' can be controlled by gpio dvs.
> +- s5m8767,pmic-buck4-uses-gpio-dvs: 'buck4' can be controlled by gpio dvs.
> +
> +Additional properties required if either of the optional properties are used:
> +
> +- s5m8767,pmic-buck234-default-dvs-idx: Default voltage setting selected from
> +  the possible 8 options selectable by the dvs gpios. The value of this
> +  property should be between 0 and 7. If not specified or if out of range, the
> +  default value of this property is set to 0.
> +
> +- s5m8767,pmic-buck-dvs-gpios: GPIO specifiers for three host gpio's used
> +  for dvs. The format of the gpio specifier depends in the gpio controller.
> +
> +Regulators: The regulators of s5m8767 that have to be instantiated should be
> +included in a sub-node named 'regulators'. Regulator nodes included in this
> +sub-node should be of the format as listed below.
> +
> +       regulator_name {
> +               standard regulator bindings here
> +       };
> +
> +The following are the names of the regulators that the s5m8767 pmic block
> +supports. Note: The 'n' in LDOn and BUCKn represents the LDO or BUCK number
> +as per the datasheet of s5m8767.
> +
> +       - LDOn
> +                 - valid values for n are 1 to 28
> +                 - Example: LDO0, LD01, LDO28
> +       - BUCKn
> +                 - valid values for n are 1 to 9.
> +                 - Example: BUCK1, BUCK2, BUCK9
> +
> +The bindings inside the regulator nodes use the standard regulator bindings
> +which are documented elsewhere.
> +
> +Example:
> +
> +       s5m8767_pmic@66 {
> +               compatible = "samsung,s5m8767-pmic";
> +               reg = <0x66>;
> +
> +               s5m8767,pmic-buck2-uses-gpio-dvs;
> +               s5m8767,pmic-buck3-uses-gpio-dvs;
> +               s5m8767,pmic-buck4-uses-gpio-dvs;
> +
> +               s5m8767,pmic-buck-default-dvs-idx = <0>;
> +
> +               s5m8767,pmic-buck-dvs-gpios = <&gpx0 0 1 0 0>, /* DVS1 */
> +                                                <&gpx0 1 1 0 0>, /* DVS2 */
> +                                                <&gpx0 2 1 0 0>; /* DVS3 */
> +
> +               s5m8767,pmic-buck-ds-gpios = <&gpx2 3 1 0 0>, /* SET1 */
> +                                               <&gpx2 4 1 0 0>, /* SET2 */
> +                                               <&gpx2 5 1 0 0>; /* SET3 */
> +
> +               s5m8767,pmic-buck2-dvs-voltage = <1350000>, <1300000>,
> +                                                <1250000>, <1200000>,
> +                                                <1150000>, <1100000>,
> +                                                <1000000>, <950000>;
> +
> +               s5m8767,pmic-buck3-dvs-voltage = <1100000>, <1100000>,
> +                                                <1100000>, <1100000>,
> +                                                <1000000>, <1000000>,
> +                                                <1000000>, <1000000>;
> +
> +               s5m8767,pmic-buck4-dvs-voltage = <1200000>, <1200000>,
> +                                                <1200000>, <1200000>,
> +                                                <1200000>, <1200000>,
> +                                                <1200000>, <1200000>;
> +
> +               regulators {
> +                       ldo1_reg: LDO1 {
> +                               regulator-name = "VDD_ABB_3.3V";
> +                               regulator-min-microvolt = <3300000>;
> +                               regulator-max-microvolt = <3300000>;
> +                       };
> +
> +                       ldo2_reg: LDO2 {
> +                               regulator-name = "VDD_ALIVE_1.1V";
> +                               regulator-min-microvolt = <1100000>;
> +                               regulator-max-microvolt = <1100000>;
> +                               regulator-always-on;
> +                       };
> +
> +                       buck1_reg: BUCK1 {
> +                               regulator-name = "VDD_MIF_1.2V";
> +                               regulator-min-microvolt = <950000>;
> +                               regulator-max-microvolt = <1350000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                       };
> +               };
> +       };
> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> index 49d361a..51de531 100644
> --- a/drivers/mfd/sec-core.c
> +++ b/drivers/mfd/sec-core.c
> @@ -17,6 +17,7 @@
>  #include <linux/err.h>
>  #include <linux/slab.h>
>  #include <linux/i2c.h>
> +#include <linux/of_irq.h>
>  #include <linux/interrupt.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/mutex.h>
> @@ -60,6 +61,15 @@ static struct mfd_cell s2mps11_devs[] = {
>         },
>  };
>
> +#ifdef CONFIG_OF
> +static struct of_device_id __devinitdata sec_dt_match[] = {
> +       {       .compatible = "samsung,s5m8767-pmic",
> +               .data = S5M8767X,
> +       },
> +       {},
> +};
> +#endif
> +
>  int sec_reg_read(struct sec_pmic_dev *sec_pmic, u8 reg, void *dest)
>  {
>         return regmap_read(sec_pmic->regmap, reg, dest);
> @@ -95,6 +105,57 @@ static struct regmap_config sec_regmap_config = {
>         .val_bits = 8,
>  };
>
> +
> +#ifdef CONFIG_OF
> +/*
> + * Only the common platform data elements for s5m8767 are parsed here from the
> + * device tree. Other sub-modules of s5m8767 such as pmic, rtc , charger and
> + * others have to parse their own platform data elements from device tree.
> + *
> + * The s5m8767 platform data structure is instantiated here and the drivers for
> + * the sub-modules need not instantiate another instance while parsing their
> + * platform data.
> + */
> +static struct sec_platform_data *sec_pmic_i2c_parse_dt_pdata(
> +                                       struct device *dev)
> +{
> +       struct sec_platform_data *pd;
> +
> +       pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> +       if (!pd) {
> +               dev_err(dev, "could not allocate memory for pdata\n");
> +               return ERR_PTR(-ENOMEM);
> +       }
> +
> +       /*
> +        * ToDo: the 'wakeup' member in the platform data is more of a linux
> +        * specfic information. Hence, there is no binding for that yet and
> +        * not parsed here.
> +        */
> +
> +       return pd;
> +}
> +#else
> +static struct sec_platform_data *sec_i2c_parse_dt_pdata(
> +                                       struct device *dev)
> +{
> +       return 0;
> +}
> +#endif
> +
> +static inline int sec_i2c_get_driver_data(struct i2c_client *i2c,
> +                                               const struct i2c_device_id *id)
> +{
> +#ifdef CONFIG_OF
> +       if (i2c->dev.of_node) {
> +               const struct of_device_id *match;
> +               match = of_match_node(sec_dt_match, i2c->dev.of_node);
> +               return (int)match->data;
> +       }
> +#endif
> +       return (int)id->driver_data;
> +}
> +
>  static int sec_pmic_probe(struct i2c_client *i2c,
>                             const struct i2c_device_id *id)
>  {
> @@ -111,13 +172,22 @@ static int sec_pmic_probe(struct i2c_client *i2c,
>         sec_pmic->dev = &i2c->dev;
>         sec_pmic->i2c = i2c;
>         sec_pmic->irq = i2c->irq;
> -       sec_pmic->type = id->driver_data;
> -
> +       sec_pmic->type = sec_i2c_get_driver_data(i2c, id);
> +
> +       if (sec_pmic->dev->of_node) {
> +               pdata = sec_pmic_i2c_parse_dt_pdata(sec_pmic->dev);
> +               if (IS_ERR(pdata)) {
> +                       ret = PTR_ERR(pdata);
> +                       return ret;
> +               }
> +               pdata->device_type = sec_pmic->type;
> +       }
>         if (pdata) {
>                 sec_pmic->device_type = pdata->device_type;
>                 sec_pmic->ono = pdata->ono;
>                 sec_pmic->irq_base = pdata->irq_base;
>                 sec_pmic->wakeup = pdata->wakeup;
> +               sec_pmic->pdata = pdata;
>         }
>
>         sec_pmic->regmap = devm_regmap_init_i2c(i2c, &sec_regmap_config);
> @@ -192,6 +262,7 @@ static struct i2c_driver sec_pmic_driver = {
>         .driver = {
>                    .name = "sec_pmic",
>                    .owner = THIS_MODULE,
> +                  .of_match_table = of_match_ptr(sec_dt_match),
>         },
>         .probe = sec_pmic_probe,
>         .remove = sec_pmic_remove,
> diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
> index 9f991f2..fcd606d 100644
> --- a/drivers/regulator/s5m8767.c
> +++ b/drivers/regulator/s5m8767.c
> @@ -14,6 +14,7 @@
>  #include <linux/bug.h>
>  #include <linux/err.h>
>  #include <linux/gpio.h>
> +#include <linux/of_gpio.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> @@ -21,6 +22,11 @@
>  #include <linux/regulator/machine.h>
>  #include <linux/mfd/samsung/core.h>
>  #include <linux/mfd/samsung/s5m8767.h>
> +#include <linux/regulator/of_regulator.h>
> +#include <linux/io.h>
> +#include <mach/map.h>
> +
> +#define S5M8767_OPMODE_NORMAL_MODE 0x1
>
>  struct s5m8767_info {
>         struct device *dev;
> @@ -508,15 +514,196 @@ static struct regulator_desc regulators[] = {
>         s5m8767_regulator_desc(BUCK9),
>  };
>
> +#ifdef CONFIG_OF
> +static int s5m8767_pmic_dt_parse_dvs_gpio(struct sec_pmic_dev *iodev,
> +                       struct sec_platform_data *pdata,
> +                       struct device_node *pmic_np)
> +{
> +       int i, gpio;
> +
> +       for (i = 0; i < 3; i++) {
> +               gpio = of_get_named_gpio(pmic_np,
> +                                       "s5m8767,pmic-buck-dvs-gpios", i);
> +               if (!gpio_is_valid(gpio)) {
> +                       dev_err(iodev->dev, "invalid gpio[%d]: %d\n", i, gpio);
> +                       return -EINVAL;
> +               }
> +               pdata->buck_gpios[i] = gpio;
> +       }
> +       return 0;
> +}
> +
> +static int s5m8767_pmic_dt_parse_ds_gpio(struct sec_pmic_dev *iodev,
> +                       struct sec_platform_data *pdata,
> +                       struct device_node *pmic_np)
> +{
> +       int i, gpio;
> +
> +       for (i = 0; i < 3; i++) {
> +               gpio = of_get_named_gpio(pmic_np,
> +                                       "s5m8767,pmic-buck-ds-gpios", i);

The binding "s5m8767,pmic-buck-ds-gpios" should be documented.

> +               if (!gpio_is_valid(gpio)) {
> +                       dev_err(iodev->dev, "invalid gpio[%d]: %d\n", i, gpio);
> +                       return -EINVAL;
> +               }
> +               pdata->buck_ds[i] = gpio;
> +       }
> +       return 0;
> +}
> +
> +static int s5m8767_pmic_dt_parse_pdata(struct sec_pmic_dev *iodev,
> +                                       struct sec_platform_data *pdata)
> +{
> +       struct device_node *pmic_np, *regulators_np, *reg_np;
> +       struct sec_regulator_data *rdata;
> +       struct sec_opmode_data *rmode;
> +       unsigned int i, dvs_voltage_nr = 1, ret;
> +
> +       pmic_np = iodev->dev->of_node;
> +       if (!pmic_np) {
> +               dev_err(iodev->dev, "could not find pmic sub-node\n");
> +               return -ENODEV;
> +       }
> +
> +       regulators_np = of_find_node_by_name(pmic_np, "regulators");
> +       if (!regulators_np) {
> +               dev_err(iodev->dev, "could not find regulators sub-node\n");
> +               return -EINVAL;
> +       }
> +
> +       /* count the number of regulators to be supported in pmic */
> +       pdata->num_regulators = 0;
> +       for_each_child_of_node(regulators_np, reg_np) {
> +               pdata->num_regulators++;
> +       }

{ } can be dropped.

> +
> +       rdata = devm_kzalloc(iodev->dev, sizeof(*rdata) *
> +                               pdata->num_regulators, GFP_KERNEL);
> +       if (!rdata) {
> +               dev_err(iodev->dev,
> +                       "could not allocate memory for regulator data\n");
> +               return -ENOMEM;
> +       }
> +
> +       rmode = devm_kzalloc(iodev->dev, sizeof(*rmode) *
> +                               pdata->num_regulators, GFP_KERNEL);
> +       if (!rdata) {
> +               dev_err(iodev->dev,
> +                       "could not allocate memory for regulator mode\n");
> +               return -ENOMEM;
> +       }
> +
> +

Additional blank line here can be removed.

> +       pdata->regulators = rdata;
> +       pdata->opmode = rmode;
> +       for_each_child_of_node(regulators_np, reg_np) {
> +               for (i = 0; i < ARRAY_SIZE(regulators); i++)
> +                       if (!of_node_cmp(reg_np->name, regulators[i].name))
> +                               break;
> +
> +               if (i == ARRAY_SIZE(regulators)) {
> +                       dev_warn(iodev->dev,
> +                       "don't know how to configure regulator %s\n",
> +                       reg_np->name);
> +                       continue;
> +               }
> +
> +               rdata->id = i;
> +               rdata->initdata = of_get_regulator_init_data(
> +                                               iodev->dev, reg_np);
> +               rdata->reg_node = reg_np;
> +               rdata++;
> +               rmode->id = i;
> +               if (of_property_read_u32(reg_np, "op_mode",
> +                               &rmode->mode)) {

"op_mode" binding needs to be documented. And rmode->mode is an 'int'.
So it is better to use a temporary u32 variable to read the value of
op_mode. And assign the value of temporary variable back to
rmode->mode. Or use of_property_read_u16 api but that is still not
right since "rmode->mode" is signed value.

> +                       dev_warn(iodev->dev, "no op_mode property property at %s\n",
> +                               reg_np->full_name);
> +
> +                       rmode->mode = S5M8767_OPMODE_NORMAL_MODE;
> +               }
> +               rmode++;
> +       }
> +
> +       if (of_get_property(pmic_np, "s5m8767,pmic-buck2-uses-gpio-dvs", NULL))
> +               pdata->buck2_gpiodvs = true;
> +
> +       if (of_get_property(pmic_np, "s5m8767,pmic-buck3-uses-gpio-dvs", NULL))
> +               pdata->buck3_gpiodvs = true;
> +
> +       if (of_get_property(pmic_np, "s5m8767,pmic-buck4-uses-gpio-dvs", NULL))
> +               pdata->buck4_gpiodvs = true;
> +
> +       if (pdata->buck2_gpiodvs || pdata->buck3_gpiodvs ||
> +                                               pdata->buck4_gpiodvs) {
> +               ret = s5m8767_pmic_dt_parse_dvs_gpio(iodev, pdata, pmic_np);
> +               if (ret)
> +                       return -EINVAL;
> +
> +               if (of_property_read_u32(pmic_np,
> +                               "s5m8767,pmic-buck-default-dvs-idx",
> +                               &pdata->buck_default_idx)) {
> +                       pdata->buck_default_idx = 0;
> +               } else {
> +                       if (pdata->buck_default_idx >= 8) {
> +                               pdata->buck_default_idx = 0;
> +                               dev_info(iodev->dev,
> +                               "invalid value for default dvs index, use 0\n");
> +                       }
> +               }
> +
> +               dvs_voltage_nr = 8;
> +       }
> +
> +       ret = s5m8767_pmic_dt_parse_ds_gpio(iodev, pdata, pmic_np);
> +       if (ret)
> +               return -EINVAL;
> +
> +       if (of_property_read_u32_array(pmic_np,
> +                               "s5m8767,pmic-buck2-dvs-voltage",
> +                               pdata->buck2_voltage, dvs_voltage_nr)) {

Same here. Maybe it is better to change pdata->buck2/3/4_voltage to
u32 data type.

> +               dev_err(iodev->dev, "buck2 voltages not specified\n");
> +               return -EINVAL;
> +       }
> +
> +       if (of_property_read_u32_array(pmic_np,
> +                               "s5m8767,pmic-buck3-dvs-voltage",
> +                               pdata->buck3_voltage, dvs_voltage_nr)) {

same here...

> +               dev_err(iodev->dev, "buck3 voltages not specified\n");
> +               return -EINVAL;
> +       }
> +
> +       if (of_property_read_u32_array(pmic_np,
> +                               "s5m8767,pmic-buck4-dvs-voltage",
> +                               pdata->buck4_voltage, dvs_voltage_nr)) {

same here...

> +               dev_err(iodev->dev, "buck4 voltages not specified\n");
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +#else
> +static int s5m8767_pmic_dt_parse_pdata(struct sec_pmic_dev *iodev,
> +                                       struct sec_platform_data *pdata)
> +{
> +       return 0;
> +}
> +#endif /* CONFIG_OF */
> +
>  static int s5m8767_pmic_probe(struct platform_device *pdev)
>  {
>         struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> -       struct sec_platform_data *pdata = dev_get_platdata(iodev->dev);
> +       struct sec_platform_data *pdata = iodev->pdata;
>         struct regulator_config config = { };
>         struct regulator_dev **rdev;
>         struct s5m8767_info *s5m8767;
>         int i, ret, size, buck_init;
>
> +       if (iodev->dev->of_node) {
> +               ret = s5m8767_pmic_dt_parse_pdata(iodev, pdata);
> +               if (ret)
> +                       return ret;
> +       }
> +
>         if (!pdata) {
>                 dev_err(pdev->dev.parent, "Platform data not supplied\n");
>                 return -ENODEV;
> @@ -765,6 +952,7 @@ static int s5m8767_pmic_probe(struct platform_device *pdev)
>                 config.dev = s5m8767->dev;
>                 config.init_data = pdata->regulators[i].initdata;
>                 config.driver_data = s5m8767;
> +               config.of_node = pdata->regulators[i].reg_node;
>
>                 rdev[i] = regulator_register(&regulators[id], &config);
>                 if (IS_ERR(rdev[i])) {
> diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
> index b50c38f..380ed95 100644
> --- a/include/linux/mfd/samsung/core.h
> +++ b/include/linux/mfd/samsung/core.h
> @@ -26,6 +26,7 @@ enum sec_device_type {
>  /**
>   * struct sec_pmic_dev - s5m87xx master device for sub-drivers
>   * @dev: master device of the chip (can be used to access platform data)
> + * @pdata: pointer to private data used to pass platform data to child
>   * @i2c: i2c client private data for regulator
>   * @rtc: i2c client private data for rtc
>   * @iolock: mutex for serializing io access
> @@ -39,6 +40,7 @@ enum sec_device_type {
>   */
>  struct sec_pmic_dev {
>         struct device *dev;
> +       struct sec_platform_data *pdata;
>         struct regmap *regmap;
>         struct i2c_client *i2c;
>         struct i2c_client *rtc;
> @@ -127,6 +129,7 @@ struct sec_platform_data {
>  struct sec_regulator_data {
>         int                             id;
>         struct regulator_init_data      *initdata;
> +       struct device_node *reg_node;
>  };
>
>  /*
> --
> 1.7.1
>

After fixing these, you could add:
Reviewed-by: Thomas Abraham <thomas.abraham@linaro.org>

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

end of thread, other threads:[~2013-01-08 22:07 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-10 12:49 [PATCH 0/4] regulator: s5m8767: Small fixes and device support Amit Daniel Kachhap
2012-12-10 12:49 ` [PATCH 1/4] regulator: s5m8767: Fix to work when platform registers less regulators Amit Daniel Kachhap
2012-12-11  3:44   ` Mark Brown
2012-12-10 12:49 ` [PATCH 2/4] regulator: s5m8767: Fix to read the first DVS register Amit Daniel Kachhap
2012-12-11  3:47   ` Mark Brown
2012-12-10 12:49 ` [PATCH 3/4] regulator: s5m8767: Fix to work even if no DVS gpio present Amit Daniel Kachhap
2012-12-11  3:48   ` Mark Brown
2012-12-10 12:49 ` [PATCH 4/4] regulator: add device tree support for s5m8767 Amit Daniel Kachhap
2013-01-08 22:07   ` Thomas Abraham

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).