devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] mfd/regulator: s5m: Various fixes and GPIO control over Buck9
@ 2013-12-17 13:23 Krzysztof Kozlowski
  2013-12-17 13:23 ` [PATCH 1/5] regulator: s5m8767: Add symbols for hard-coded DVS_RAMP register Krzysztof Kozlowski
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2013-12-17 13:23 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Mark Brown, Grant Likely, Sachin Kamat, Thomas Abraham,
	Amit Daniel Kachhap, Krzysztof Kozlowski, devicetree, linux-doc,
	linux-kernel

Hi,

This is a small set of patches with minor fixes for s5m8767 MFD driver
and regulator.

The patchset touches 3 different issues:
1. Patch 1/5: Add symbols for hard-coded DVS_RAMP values.
2. Patch 2/5 and 3/5: Remove the sec_reg* helpers hiding the regmap API.
3. Patch 4/5 and 5/5: Add GPIO control over Buck9/eMMC regulator.

I am sending this in one set as all of the patches use regmap API and they
mix over the removal of sec_reg* helpers.

The patchset is based on v3.13-rc4 however they also depend on previous
s5m8767 regulator patches (already in Mark Brown tree):
 - regulator: s5m8767: Define symbol for buck control mask
 - regulator: s5m8767: Implement voltage setting for BUCK7/8 regulators
https://git.kernel.org/cgit/linux/kernel/git/broonie/regulator.git/log/?h=topic/s5m8767


Best regards,
Krzysztof


Krzysztof Kozlowski (5):
  regulator: s5m8767: Add symbols for hard-coded DVS_RAMP register
  regulator: s5m8767: Do not use sec_reg* regmap helpers
  mfd: sec: Remove sec_reg* regmap helpers
  regulator: s5m8767: Use GPIO for controlling Buck9/eMMC
  regulator: s5m8767: Document new bindings for Buck9 GPIO control

 .../bindings/regulator/s5m8767-regulator.txt       |   16 ++
 drivers/mfd/sec-core.c                             |   30 ---
 drivers/regulator/s5m8767.c                        |  207 ++++++++++++++++----
 include/linux/mfd/samsung/core.h                   |    8 +-
 include/linux/mfd/samsung/s5m8767.h                |   22 +++
 5 files changed, 207 insertions(+), 76 deletions(-)

-- 
1.7.9.5


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

* [PATCH 1/5] regulator: s5m8767: Add symbols for hard-coded DVS_RAMP register
  2013-12-17 13:23 [PATCH 0/5] mfd/regulator: s5m: Various fixes and GPIO control over Buck9 Krzysztof Kozlowski
@ 2013-12-17 13:23 ` Krzysztof Kozlowski
  2013-12-17 13:51   ` Lee Jones
                     ` (2 more replies)
       [not found] ` <1387286601-21646-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2013-12-17 13:23 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Mark Brown, Grant Likely, Sachin Kamat, Thomas Abraham,
	Amit Daniel Kachhap, Krzysztof Kozlowski, devicetree, linux-doc,
	linux-kernel
  Cc: Kyungmin Park, Marek Szyprowski

Add symbols for hard-coded values of BUCK_RAMP field in DVS_RAMP
register. This simplifies a little the code as register update is called
only once.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/regulator/s5m8767.c         |   22 ++++++++++------------
 include/linux/mfd/samsung/s5m8767.h |   15 +++++++++++++++
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
index 397917611ecd..10b4b6a5b08e 100644
--- a/drivers/regulator/s5m8767.c
+++ b/drivers/regulator/s5m8767.c
@@ -881,31 +881,29 @@ static int s5m8767_pmic_probe(struct platform_device *pdev)
 
 	if (s5m8767->buck2_ramp || s5m8767->buck3_ramp
 		|| s5m8767->buck4_ramp) {
+		unsigned int val;
 		switch (s5m8767->ramp_delay) {
 		case 5:
-			sec_reg_update(s5m8767->iodev, S5M8767_REG_DVSRAMP,
-					0x40, 0xf0);
+			val = S5M8767_DVS_BUCK_RAMP_5;
 			break;
 		case 10:
-			sec_reg_update(s5m8767->iodev, S5M8767_REG_DVSRAMP,
-					0x90, 0xf0);
+			val = S5M8767_DVS_BUCK_RAMP_10;
 			break;
 		case 25:
-			sec_reg_update(s5m8767->iodev, S5M8767_REG_DVSRAMP,
-					0xd0, 0xf0);
+			val = S5M8767_DVS_BUCK_RAMP_25;
 			break;
 		case 50:
-			sec_reg_update(s5m8767->iodev, S5M8767_REG_DVSRAMP,
-					0xe0, 0xf0);
+			val = S5M8767_DVS_BUCK_RAMP_50;
 			break;
 		case 100:
-			sec_reg_update(s5m8767->iodev, S5M8767_REG_DVSRAMP,
-					0xf0, 0xf0);
+			val = S5M8767_DVS_BUCK_RAMP_100;
 			break;
 		default:
-			sec_reg_update(s5m8767->iodev, S5M8767_REG_DVSRAMP,
-					0x90, 0xf0);
+			val = S5M8767_DVS_BUCK_RAMP_10;
 		}
+		sec_reg_update(s5m8767->iodev, S5M8767_REG_DVSRAMP,
+					val << S5M8767_DVS_BUCK_RAMP_SHIFT,
+					S5M8767_DVS_BUCK_RAMP_MASK);
 	}
 
 	for (i = 0; i < pdata->num_regulators; i++) {
diff --git a/include/linux/mfd/samsung/s5m8767.h b/include/linux/mfd/samsung/s5m8767.h
index 9198377ee859..2ab0b0f03641 100644
--- a/include/linux/mfd/samsung/s5m8767.h
+++ b/include/linux/mfd/samsung/s5m8767.h
@@ -186,4 +186,19 @@ enum s5m8767_regulators {
 #define S5M8767_ENCTRL_SHIFT		6
 #define S5M8767_ENCTRL_MASK		(0x3 << S5M8767_ENCTRL_SHIFT)
 
+/*
+ * Values for BUCK_RAMP field in DVS_RAMP register, matching raw values
+ * in mV/us.
+ */
+enum s5m8767_dvs_buck_ramp_values {
+	S5M8767_DVS_BUCK_RAMP_5		= 0x4,
+	S5M8767_DVS_BUCK_RAMP_10	= 0x9,
+	S5M8767_DVS_BUCK_RAMP_12_5	= 0xb,
+	S5M8767_DVS_BUCK_RAMP_25	= 0xd,
+	S5M8767_DVS_BUCK_RAMP_50	= 0xe,
+	S5M8767_DVS_BUCK_RAMP_100	= 0xf,
+};
+#define S5M8767_DVS_BUCK_RAMP_SHIFT	4
+#define S5M8767_DVS_BUCK_RAMP_MASK	(0xf << S5M8767_DVS_BUCK_RAMP_SHIFT)
+
 #endif /* __LINUX_MFD_S5M8767_H */
-- 
1.7.9.5


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

* [PATCH 2/5] regulator: s5m8767: Do not use sec_reg* regmap helpers
       [not found] ` <1387286601-21646-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@ 2013-12-17 13:23   ` Krzysztof Kozlowski
  2013-12-17 15:03     ` Lee Jones
       [not found]     ` <1387286601-21646-3-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
  2013-12-17 13:23   ` [PATCH 5/5] regulator: s5m8767: Document new bindings for Buck9 GPIO control Krzysztof Kozlowski
  1 sibling, 2 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2013-12-17 13:23 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Mark Brown, Grant Likely, Sachin Kamat, Thomas Abraham,
	Amit Daniel Kachhap, Krzysztof Kozlowski,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Kyungmin Park, Marek Szyprowski

Replace calls to sec_reg* helpers with direct usage of regmap API. The
sec_reg* helpers are error-prone as they mix u8 with unsigned int and
order of some of parameters (val and mask in sec_reg_update()).

Also the helpers do not give any way of useful abstraction as they just
call corresponding regmap function.

This patch replaces:
 - sec_reg_read() with regmap_read(),
 - sec_reg_write() with regmap_write(),
 - sec_reg_update() with regmap_update_bits().

Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/regulator/s5m8767.c |   71 +++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 30 deletions(-)

diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
index 10b4b6a5b08e..d7164bb75d3e 100644
--- a/drivers/regulator/s5m8767.c
+++ b/drivers/regulator/s5m8767.c
@@ -23,6 +23,7 @@
 #include <linux/mfd/samsung/core.h>
 #include <linux/mfd/samsung/s5m8767.h>
 #include <linux/regulator/of_regulator.h>
+#include <linux/regmap.h>
 
 #define S5M8767_OPMODE_NORMAL_MODE 0x1
 
@@ -226,7 +227,7 @@ static int s5m8767_reg_is_enabled(struct regulator_dev *rdev)
 	else if (ret)
 		return ret;
 
-	ret = sec_reg_read(s5m8767->iodev, reg, &val);
+	ret = regmap_read(s5m8767->iodev->regmap_pmic, reg, &val);
 	if (ret)
 		return ret;
 
@@ -243,21 +244,21 @@ static int s5m8767_reg_enable(struct regulator_dev *rdev)
 	if (ret)
 		return ret;
 
-	return sec_reg_update(s5m8767->iodev, reg, enable_ctrl,
-			S5M8767_ENCTRL_MASK);
+	return regmap_update_bits(s5m8767->iodev->regmap_pmic, reg,
+			S5M8767_ENCTRL_MASK, enable_ctrl);
 }
 
 static int s5m8767_reg_disable(struct regulator_dev *rdev)
 {
 	struct s5m8767_info *s5m8767 = rdev_get_drvdata(rdev);
-	int ret, reg;
-	int mask = S5M8767_ENCTRL_MASK, enable_ctrl;
+	int ret, reg, enable_ctrl;
 
 	ret = s5m8767_get_register(rdev, &reg, &enable_ctrl);
 	if (ret)
 		return ret;
 
-	return sec_reg_update(s5m8767->iodev, reg, ~mask, mask);
+	return regmap_update_bits(s5m8767->iodev->regmap_pmic, reg,
+			S5M8767_ENCTRL_MASK, ~S5M8767_ENCTRL_MASK);
 }
 
 static int s5m8767_get_vsel_reg(int reg_id, struct s5m8767_info *s5m8767)
@@ -749,17 +750,20 @@ static int s5m8767_pmic_probe(struct platform_device *pdev)
 	buck_init = s5m8767_convert_voltage_to_sel(&buck_voltage_val2,
 						   pdata->buck2_init);
 
-	sec_reg_write(s5m8767->iodev, S5M8767_REG_BUCK2DVS2, buck_init);
+	regmap_write(s5m8767->iodev->regmap_pmic, S5M8767_REG_BUCK2DVS2,
+			buck_init);
 
 	buck_init = s5m8767_convert_voltage_to_sel(&buck_voltage_val2,
 						   pdata->buck3_init);
 
-	sec_reg_write(s5m8767->iodev, S5M8767_REG_BUCK3DVS2, buck_init);
+	regmap_write(s5m8767->iodev->regmap_pmic, S5M8767_REG_BUCK3DVS2,
+			buck_init);
 
 	buck_init = s5m8767_convert_voltage_to_sel(&buck_voltage_val2,
 						   pdata->buck4_init);
 
-	sec_reg_write(s5m8767->iodev, S5M8767_REG_BUCK4DVS2, buck_init);
+	regmap_write(s5m8767->iodev->regmap_pmic, S5M8767_REG_BUCK4DVS2,
+			buck_init);
 
 	for (i = 0; i < 8; i++) {
 		if (s5m8767->buck2_gpiodvs) {
@@ -841,43 +845,49 @@ static int s5m8767_pmic_probe(struct platform_device *pdev)
 
 	if (pdata->buck2_gpiodvs || pdata->buck3_gpiodvs ||
 	   pdata->buck4_gpiodvs) {
-		sec_reg_update(s5m8767->iodev, S5M8767_REG_BUCK2CTRL,
-				(pdata->buck2_gpiodvs) ? (1 << 1) : (0 << 1),
-				1 << 1);
-		sec_reg_update(s5m8767->iodev, S5M8767_REG_BUCK3CTRL,
-				(pdata->buck3_gpiodvs) ? (1 << 1) : (0 << 1),
-				1 << 1);
-		sec_reg_update(s5m8767->iodev, S5M8767_REG_BUCK4CTRL,
-				(pdata->buck4_gpiodvs) ? (1 << 1) : (0 << 1),
-				1 << 1);
+		regmap_update_bits(s5m8767->iodev->regmap_pmic,
+				S5M8767_REG_BUCK2CTRL, 1 << 1,
+				(pdata->buck2_gpiodvs) ? (1 << 1) : (0 << 1));
+		regmap_update_bits(s5m8767->iodev->regmap_pmic,
+				S5M8767_REG_BUCK3CTRL, 1 << 1,
+				(pdata->buck3_gpiodvs) ? (1 << 1) : (0 << 1));
+		regmap_update_bits(s5m8767->iodev->regmap_pmic,
+				S5M8767_REG_BUCK4CTRL, 1 << 1,
+				(pdata->buck4_gpiodvs) ? (1 << 1) : (0 << 1));
 	}
 
 	/* Initialize GPIO DVS registers */
 	for (i = 0; i < 8; i++) {
 		if (s5m8767->buck2_gpiodvs) {
-			sec_reg_write(s5m8767->iodev, S5M8767_REG_BUCK2DVS1 + i,
-					   s5m8767->buck2_vol[i]);
+			regmap_write(s5m8767->iodev->regmap_pmic,
+					S5M8767_REG_BUCK2DVS1 + i,
+					s5m8767->buck2_vol[i]);
 		}
 
 		if (s5m8767->buck3_gpiodvs) {
-			sec_reg_write(s5m8767->iodev, S5M8767_REG_BUCK3DVS1 + i,
-					   s5m8767->buck3_vol[i]);
+			regmap_write(s5m8767->iodev->regmap_pmic,
+					S5M8767_REG_BUCK3DVS1 + i,
+					s5m8767->buck3_vol[i]);
 		}
 
 		if (s5m8767->buck4_gpiodvs) {
-			sec_reg_write(s5m8767->iodev, S5M8767_REG_BUCK4DVS1 + i,
-					   s5m8767->buck4_vol[i]);
+			regmap_write(s5m8767->iodev->regmap_pmic,
+					S5M8767_REG_BUCK4DVS1 + i,
+					s5m8767->buck4_vol[i]);
 		}
 	}
 
 	if (s5m8767->buck2_ramp)
-		sec_reg_update(s5m8767->iodev, S5M8767_REG_DVSRAMP, 0x08, 0x08);
+		regmap_update_bits(s5m8767->iodev->regmap_pmic,
+				S5M8767_REG_DVSRAMP, 0x08, 0x08);
 
 	if (s5m8767->buck3_ramp)
-		sec_reg_update(s5m8767->iodev, S5M8767_REG_DVSRAMP, 0x04, 0x04);
+		regmap_update_bits(s5m8767->iodev->regmap_pmic,
+				S5M8767_REG_DVSRAMP, 0x04, 0x04);
 
 	if (s5m8767->buck4_ramp)
-		sec_reg_update(s5m8767->iodev, S5M8767_REG_DVSRAMP, 0x02, 0x02);
+		regmap_update_bits(s5m8767->iodev->regmap_pmic,
+				S5M8767_REG_DVSRAMP, 0x02, 0x02);
 
 	if (s5m8767->buck2_ramp || s5m8767->buck3_ramp
 		|| s5m8767->buck4_ramp) {
@@ -901,9 +911,10 @@ static int s5m8767_pmic_probe(struct platform_device *pdev)
 		default:
 			val = S5M8767_DVS_BUCK_RAMP_10;
 		}
-		sec_reg_update(s5m8767->iodev, S5M8767_REG_DVSRAMP,
-					val << S5M8767_DVS_BUCK_RAMP_SHIFT,
-					S5M8767_DVS_BUCK_RAMP_MASK);
+		regmap_update_bits(s5m8767->iodev->regmap_pmic,
+					S5M8767_REG_DVSRAMP,
+					S5M8767_DVS_BUCK_RAMP_MASK,
+					val << S5M8767_DVS_BUCK_RAMP_SHIFT);
 	}
 
 	for (i = 0; i < pdata->num_regulators; i++) {
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 3/5] mfd: sec: Remove sec_reg* regmap helpers
  2013-12-17 13:23 [PATCH 0/5] mfd/regulator: s5m: Various fixes and GPIO control over Buck9 Krzysztof Kozlowski
  2013-12-17 13:23 ` [PATCH 1/5] regulator: s5m8767: Add symbols for hard-coded DVS_RAMP register Krzysztof Kozlowski
       [not found] ` <1387286601-21646-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@ 2013-12-17 13:23 ` Krzysztof Kozlowski
  2013-12-18 13:36   ` Mark Brown
  2013-12-17 13:23 ` [PATCH 4/5] regulator: s5m8767: Use GPIO for controlling Buck9/eMMC Krzysztof Kozlowski
  3 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2013-12-17 13:23 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Mark Brown, Grant Likely, Sachin Kamat, Thomas Abraham,
	Amit Daniel Kachhap, Krzysztof Kozlowski, devicetree, linux-doc,
	linux-kernel
  Cc: Kyungmin Park, Marek Szyprowski

Remove sec_reg* helpers as they are not used anymore. These helpers were
error-prone as they mixed u8 with unsigned int and they changed order of
some of parameters (val and mask in sec_reg_update()).

Also the helpers didn't give any way of useful abstraction as they just
called corresponding regmap function.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/mfd/sec-core.c           |   30 ------------------------------
 include/linux/mfd/samsung/core.h |    6 ------
 2 files changed, 36 deletions(-)

diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
index 83f31e2e5754..ab4c77b01f9f 100644
--- a/drivers/mfd/sec-core.c
+++ b/drivers/mfd/sec-core.c
@@ -79,36 +79,6 @@ static struct of_device_id sec_dt_match[] = {
 };
 #endif
 
-int sec_reg_read(struct sec_pmic_dev *sec_pmic, u8 reg, void *dest)
-{
-	return regmap_read(sec_pmic->regmap_pmic, reg, dest);
-}
-EXPORT_SYMBOL_GPL(sec_reg_read);
-
-int sec_bulk_read(struct sec_pmic_dev *sec_pmic, u8 reg, int count, u8 *buf)
-{
-	return regmap_bulk_read(sec_pmic->regmap_pmic, reg, buf, count);
-}
-EXPORT_SYMBOL_GPL(sec_bulk_read);
-
-int sec_reg_write(struct sec_pmic_dev *sec_pmic, u8 reg, u8 value)
-{
-	return regmap_write(sec_pmic->regmap_pmic, reg, value);
-}
-EXPORT_SYMBOL_GPL(sec_reg_write);
-
-int sec_bulk_write(struct sec_pmic_dev *sec_pmic, u8 reg, int count, u8 *buf)
-{
-	return regmap_raw_write(sec_pmic->regmap_pmic, reg, buf, count);
-}
-EXPORT_SYMBOL_GPL(sec_bulk_write);
-
-int sec_reg_update(struct sec_pmic_dev *sec_pmic, u8 reg, u8 val, u8 mask)
-{
-	return regmap_update_bits(sec_pmic->regmap_pmic, reg, mask, val);
-}
-EXPORT_SYMBOL_GPL(sec_reg_update);
-
 static bool s2mps11_volatile(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
index cab2dd279076..41c9bde410c5 100644
--- a/include/linux/mfd/samsung/core.h
+++ b/include/linux/mfd/samsung/core.h
@@ -59,12 +59,6 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic);
 void sec_irq_exit(struct sec_pmic_dev *sec_pmic);
 int sec_irq_resume(struct sec_pmic_dev *sec_pmic);
 
-extern int sec_reg_read(struct sec_pmic_dev *sec_pmic, u8 reg, void *dest);
-extern int sec_bulk_read(struct sec_pmic_dev *sec_pmic, u8 reg, int count, u8 *buf);
-extern int sec_reg_write(struct sec_pmic_dev *sec_pmic, u8 reg, u8 value);
-extern int sec_bulk_write(struct sec_pmic_dev *sec_pmic, u8 reg, int count, u8 *buf);
-extern int sec_reg_update(struct sec_pmic_dev *sec_pmic, u8 reg, u8 val, u8 mask);
-
 struct sec_platform_data {
 	struct sec_regulator_data	*regulators;
 	struct sec_opmode_data		*opmode;
-- 
1.7.9.5

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

* [PATCH 4/5] regulator: s5m8767: Use GPIO for controlling Buck9/eMMC
  2013-12-17 13:23 [PATCH 0/5] mfd/regulator: s5m: Various fixes and GPIO control over Buck9 Krzysztof Kozlowski
                   ` (2 preceding siblings ...)
  2013-12-17 13:23 ` [PATCH 3/5] mfd: sec: Remove sec_reg* regmap helpers Krzysztof Kozlowski
@ 2013-12-17 13:23 ` Krzysztof Kozlowski
  2013-12-17 13:49   ` Lee Jones
  2013-12-18 13:47   ` Mark Brown
  3 siblings, 2 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2013-12-17 13:23 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Mark Brown, Grant Likely, Sachin Kamat, Thomas Abraham,
	Amit Daniel Kachhap, Krzysztof Kozlowski, devicetree, linux-doc,
	linux-kernel
  Cc: Kyungmin Park, Marek Szyprowski

Add support for GPIO control (enable/disable) over Buck9. The Buck9
Converter is used as a supply for eMMC Host Controller.

BUCK9EN GPIO of S5M8767 chip may be used by application processor to
enable or disable the Buck9. This has two benefits:
 - It is faster than toggling it over I2C bus.
 - It allows disabling the regulator during suspend to RAM; The AP will
   enable it during resume; Without the patch the regulator supplying
   eMMC must be defined as fixed-regulator.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/regulator/s5m8767.c         |  120 ++++++++++++++++++++++++++++++++++-
 include/linux/mfd/samsung/core.h    |    2 +
 include/linux/mfd/samsung/s5m8767.h |    7 ++
 3 files changed, 128 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
index d7164bb75d3e..86fb44c56bac 100644
--- a/drivers/regulator/s5m8767.c
+++ b/drivers/regulator/s5m8767.c
@@ -48,6 +48,8 @@ struct s5m8767_info {
 	int buck_gpios[3];
 	int buck_ds[3];
 	int buck_gpioindex;
+	bool buck9_uses_gpio;
+	int buck9_gpio;
 };
 
 struct sec_voltage_desc {
@@ -261,6 +263,43 @@ static int s5m8767_reg_disable(struct regulator_dev *rdev)
 			S5M8767_ENCTRL_MASK, ~S5M8767_ENCTRL_MASK);
 }
 
+static int s5m8767_reg_gpio_is_enabled(struct regulator_dev *rdev)
+{
+	struct s5m8767_info *s5m8767 = rdev_get_drvdata(rdev);
+	int val;
+
+	if (!s5m8767->buck9_uses_gpio)
+		return s5m8767_reg_is_enabled(rdev);
+
+	val = gpio_get_value(s5m8767->buck9_gpio);
+
+	return val == 1;
+}
+
+static int s5m8767_reg_gpio_enable(struct regulator_dev *rdev)
+{
+	struct s5m8767_info *s5m8767 = rdev_get_drvdata(rdev);
+
+	if (!s5m8767->buck9_uses_gpio)
+		return s5m8767_reg_enable(rdev);
+
+	gpio_set_value(s5m8767->buck9_gpio, 1);
+
+	return 0;
+}
+
+static int s5m8767_reg_gpio_disable(struct regulator_dev *rdev)
+{
+	struct s5m8767_info *s5m8767 = rdev_get_drvdata(rdev);
+
+	if (!s5m8767->buck9_uses_gpio)
+		return s5m8767_reg_disable(rdev);
+
+	gpio_set_value(s5m8767->buck9_gpio, 0);
+
+	return 0;
+}
+
 static int s5m8767_get_vsel_reg(int reg_id, struct s5m8767_info *s5m8767)
 {
 	int reg;
@@ -427,6 +466,17 @@ static struct regulator_ops s5m8767_buck78_ops = {
 	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
 };
 
+static struct regulator_ops s5m8767_buck_gpio_ops = {
+	.list_voltage		= regulator_list_voltage_linear,
+	.is_enabled		= s5m8767_reg_gpio_is_enabled,
+	.enable			= s5m8767_reg_gpio_enable,
+	.disable		= s5m8767_reg_gpio_disable,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.set_voltage_time_sel	= s5m8767_set_voltage_time_sel,
+};
+
+
 #define s5m8767_regulator_desc(_name) {		\
 	.name		= #_name,		\
 	.id		= S5M8767_##_name,	\
@@ -443,6 +493,14 @@ static struct regulator_ops s5m8767_buck78_ops = {
 	.owner		= THIS_MODULE,		\
 }
 
+#define s5m8767_regulator_gpio_desc(_name) {	\
+	.name		= #_name,		\
+	.id		= S5M8767_##_name,	\
+	.ops		= &s5m8767_buck_gpio_ops,	\
+	.type		= REGULATOR_VOLTAGE,	\
+	.owner		= THIS_MODULE,		\
+}
+
 static struct regulator_desc regulators[] = {
 	s5m8767_regulator_desc(LDO1),
 	s5m8767_regulator_desc(LDO2),
@@ -480,9 +538,50 @@ static struct regulator_desc regulators[] = {
 	s5m8767_regulator_desc(BUCK6),
 	s5m8767_regulator_buck78_desc(BUCK7),
 	s5m8767_regulator_buck78_desc(BUCK8),
-	s5m8767_regulator_desc(BUCK9),
+	s5m8767_regulator_gpio_desc(BUCK9),
 };
 
+/*
+ * Initialize BUCK9EN GPIO and BUCK9 control register.
+ * Assuming DT or platform data is already parsed and buck9_uses_gpio is true.
+ */
+static int s5m8767_init_buck9en_gpio(struct s5m8767_info *s5m8767)
+{
+	int i, ret, mode = 0;
+	unsigned int data;
+
+	/* Check if opmode for Buck9 matches pmic-buck9-uses-gpio */
+	for (i = 0; i < s5m8767->num_regulators; i++) {
+		const struct sec_opmode_data *opmode = &s5m8767->opmode[i];
+		if (opmode->id == S5M8767_BUCK9) {
+			mode = s5m8767_opmode_reg[S5M8767_BUCK9][opmode->mode];
+			break;
+		}
+	}
+	if (mode != S5M8767_ENCTRL_USE_GPIO) {
+		dev_err(s5m8767->dev,
+				"Mismatched op_mode (%x) and uses-gpio for Buck9, fallback to normal mode\n",
+				mode);
+		s5m8767->buck9_uses_gpio = false;
+		return 0;
+	}
+
+	if (!gpio_is_valid(s5m8767->buck9_gpio)) {
+		dev_err(s5m8767->dev, "Buck9 GPIO not valid\n");
+		return -EINVAL;
+	}
+
+	ret = devm_gpio_request_one(s5m8767->dev, s5m8767->buck9_gpio,
+			GPIOF_OUT_INIT_HIGH, "S5M8767 BUCK9EN");
+	if (ret)
+		return ret;
+
+	data = S5M8767_ENCTRL_USE_GPIO << S5M8767_ENCTRL_SHIFT;
+
+	return regmap_update_bits(s5m8767->iodev->regmap_pmic,
+			S5M8767_REG_BUCK9CTRL1, S5M8767_ENCTRL_MASK, data);
+}
+
 #ifdef CONFIG_OF
 static int s5m8767_pmic_dt_parse_dvs_gpio(struct sec_pmic_dev *iodev,
 			struct sec_platform_data *pdata,
@@ -663,6 +762,17 @@ static int s5m8767_pmic_dt_parse_pdata(struct platform_device *pdev,
 			pdata->buck_ramp_delay = 0;
 	}
 
+	if (of_get_property(pmic_np, "s5m8767,pmic-buck9-uses-gpio", NULL)) {
+		int gpio = of_get_named_gpio(pmic_np,
+					"s5m8767,pmic-buck9-gpio", 0);
+		if (!gpio_is_valid(gpio)) {
+			dev_err(iodev->dev, "invalid buck9 gpio: %d\n", gpio);
+			return -EINVAL;
+		}
+		pdata->buck9_uses_gpio = true;
+		pdata->buck9_gpio = gpio;
+	}
+
 	return 0;
 }
 #else
@@ -740,6 +850,8 @@ static int s5m8767_pmic_probe(struct platform_device *pdev)
 	s5m8767->buck_ds[0] = pdata->buck_ds[0];
 	s5m8767->buck_ds[1] = pdata->buck_ds[1];
 	s5m8767->buck_ds[2] = pdata->buck_ds[2];
+	s5m8767->buck9_uses_gpio = pdata->buck9_uses_gpio;
+	s5m8767->buck9_gpio = pdata->buck9_gpio;
 
 	s5m8767->ramp_delay = pdata->buck_ramp_delay;
 	s5m8767->buck2_ramp = pdata->buck2_ramp_enable;
@@ -917,6 +1029,12 @@ static int s5m8767_pmic_probe(struct platform_device *pdev)
 					val << S5M8767_DVS_BUCK_RAMP_SHIFT);
 	}
 
+	if (s5m8767->buck9_uses_gpio) {
+		ret = s5m8767_init_buck9en_gpio(s5m8767);
+		if (ret)
+			return ret;
+	}
+
 	for (i = 0; i < pdata->num_regulators; i++) {
 		const struct sec_voltage_desc *desc;
 		int id = pdata->regulators[i].id;
diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
index 41c9bde410c5..8c52ecea3f11 100644
--- a/include/linux/mfd/samsung/core.h
+++ b/include/linux/mfd/samsung/core.h
@@ -80,6 +80,8 @@ struct sec_platform_data {
 	bool				buck3_gpiodvs;
 	unsigned int			buck4_voltage[8];
 	bool				buck4_gpiodvs;
+	int				buck9_gpio;
+	bool				buck9_uses_gpio;
 
 	int				buck_set1;
 	int				buck_set2;
diff --git a/include/linux/mfd/samsung/s5m8767.h b/include/linux/mfd/samsung/s5m8767.h
index 2ab0b0f03641..d383c46a9ad8 100644
--- a/include/linux/mfd/samsung/s5m8767.h
+++ b/include/linux/mfd/samsung/s5m8767.h
@@ -183,8 +183,15 @@ enum s5m8767_regulators {
 	S5M8767_REG_MAX,
 };
 
+/* LDO_EN/BUCK_EN field in registers */
 #define S5M8767_ENCTRL_SHIFT		6
 #define S5M8767_ENCTRL_MASK		(0x3 << S5M8767_ENCTRL_SHIFT)
+/*
+ * LDO_EN/BUCK_EN register value for controlling this Buck or LDO
+ * by GPIO (PWREN, BUCKEN).
+ */
+#define S5M8767_ENCTRL_USE_GPIO		0x1
+
 
 /*
  * Values for BUCK_RAMP field in DVS_RAMP register, matching raw values
-- 
1.7.9.5


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

* [PATCH 5/5] regulator: s5m8767: Document new bindings for Buck9 GPIO control
       [not found] ` <1387286601-21646-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
  2013-12-17 13:23   ` [PATCH 2/5] regulator: s5m8767: Do not use sec_reg* regmap helpers Krzysztof Kozlowski
@ 2013-12-17 13:23   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2013-12-17 13:23 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Mark Brown, Grant Likely, Sachin Kamat, Thomas Abraham,
	Amit Daniel Kachhap, Krzysztof Kozlowski,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Kyungmin Park, Marek Szyprowski

Add documentation for new bindings for controlling (enable/disable) the
Buck9 Converter by GPIO (BUCK9EN).

Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 .../bindings/regulator/s5m8767-regulator.txt       |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt b/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
index d1660a90fc06..6a921b5582a2 100644
--- a/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
@@ -43,6 +43,7 @@ Optional properties:
 - 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.
+- s5m8767,pmic-buck9-uses-gpio: 'buck9' can be enabled/disabled by GPIO.
 
 Additional properties required if either of the optional properties are used:
 
@@ -54,6 +55,11 @@ Additional properties required if either of the optional properties are used:
 - 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.
 
+- s5m8767,pmic-buck9-gpio: GPIO specifier for one GPIO controlling buck9.
+  This property is required when 's5m8767,pmic-buck9-uses-gpio' is specified.
+  Additionally for proper GPIO control the buck9 regulator should be also
+  configured with proper opmode (low power or suspend).
+
 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.
@@ -126,6 +132,9 @@ Example:
 						 <1200000>, <1200000>,
 						 <1200000>, <1200000>;
 
+		s5m8767,pmic-buck9-uses-gpio;
+		s5m8767,pmic-buck9-gpio = <&gpk0 2 0>;
+
 		regulators {
 			ldo1_reg: LDO1 {
 				regulator-name = "VDD_ABB_3.3V";
@@ -148,5 +157,12 @@ Example:
 				regulator-always-on;
 				regulator-boot-on;
 			};
+
+			vemmc_reg: BUCK9 {
+				regulator-name = "VMEM_VDD_2.8V";
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				op_mode = <3>; /* Standby Mode */
+			};
 		};
 	};
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 4/5] regulator: s5m8767: Use GPIO for controlling Buck9/eMMC
  2013-12-17 13:23 ` [PATCH 4/5] regulator: s5m8767: Use GPIO for controlling Buck9/eMMC Krzysztof Kozlowski
@ 2013-12-17 13:49   ` Lee Jones
  2013-12-18 13:47   ` Mark Brown
  1 sibling, 0 replies; 19+ messages in thread
From: Lee Jones @ 2013-12-17 13:49 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Liam Girdwood,
	Mark Brown, Grant Likely, Sachin Kamat, Thomas Abraham,
	Amit Daniel Kachhap, devicetree, linux-doc, linux-kernel,
	Kyungmin Park, Marek Szyprowski

On Tue, 17 Dec 2013, Krzysztof Kozlowski wrote:

> Add support for GPIO control (enable/disable) over Buck9. The Buck9
> Converter is used as a supply for eMMC Host Controller.
> 
> BUCK9EN GPIO of S5M8767 chip may be used by application processor to
> enable or disable the Buck9. This has two benefits:
>  - It is faster than toggling it over I2C bus.
>  - It allows disabling the regulator during suspend to RAM; The AP will
>    enable it during resume; Without the patch the regulator supplying
>    eMMC must be defined as fixed-regulator.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/regulator/s5m8767.c         |  120 ++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/samsung/core.h    |    2 +
>  include/linux/mfd/samsung/s5m8767.h |    7 ++

So long as they fit in with the rest of the changes, the MFD adaptions
look fine to me:

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 1/5] regulator: s5m8767: Add symbols for hard-coded DVS_RAMP register
  2013-12-17 13:23 ` [PATCH 1/5] regulator: s5m8767: Add symbols for hard-coded DVS_RAMP register Krzysztof Kozlowski
@ 2013-12-17 13:51   ` Lee Jones
  2013-12-17 14:06     ` Krzysztof Kozlowski
  2013-12-18 13:27   ` Mark Brown
  2013-12-18 13:28   ` Mark Brown
  2 siblings, 1 reply; 19+ messages in thread
From: Lee Jones @ 2013-12-17 13:51 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Liam Girdwood,
	Mark Brown, Grant Likely, Sachin Kamat, Thomas Abraham,
	Amit Daniel Kachhap, devicetree, linux-doc, linux-kernel,
	Kyungmin Park, Marek Szyprowski

> Add symbols for hard-coded values of BUCK_RAMP field in DVS_RAMP
> register. This simplifies a little the code as register update is called
> only once.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/regulator/s5m8767.c         |   22 ++++++++++------------
>  include/linux/mfd/samsung/s5m8767.h |   15 +++++++++++++++
>  2 files changed, 25 insertions(+), 12 deletions(-)

<snip>

> diff --git a/include/linux/mfd/samsung/s5m8767.h b/include/linux/mfd/samsung/s5m8767.h
> index 9198377ee859..2ab0b0f03641 100644
> --- a/include/linux/mfd/samsung/s5m8767.h
> +++ b/include/linux/mfd/samsung/s5m8767.h
> @@ -186,4 +186,19 @@ enum s5m8767_regulators {
>  #define S5M8767_ENCTRL_SHIFT		6
>  #define S5M8767_ENCTRL_MASK		(0x3 << S5M8767_ENCTRL_SHIFT)
>  
> +/*
> + * Values for BUCK_RAMP field in DVS_RAMP register, matching raw values
> + * in mV/us.
> + */
> +enum s5m8767_dvs_buck_ramp_values {
> +	S5M8767_DVS_BUCK_RAMP_5		= 0x4,
> +	S5M8767_DVS_BUCK_RAMP_10	= 0x9,
> +	S5M8767_DVS_BUCK_RAMP_12_5	= 0xb,
> +	S5M8767_DVS_BUCK_RAMP_25	= 0xd,
> +	S5M8767_DVS_BUCK_RAMP_50	= 0xe,
> +	S5M8767_DVS_BUCK_RAMP_100	= 0xf,
> +};

Why do these have to be enums?

> +#define S5M8767_DVS_BUCK_RAMP_SHIFT	4
> +#define S5M8767_DVS_BUCK_RAMP_MASK	(0xf << S5M8767_DVS_BUCK_RAMP_SHIFT)
> +
>  #endif /* __LINUX_MFD_S5M8767_H */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 1/5] regulator: s5m8767: Add symbols for hard-coded DVS_RAMP register
  2013-12-17 13:51   ` Lee Jones
@ 2013-12-17 14:06     ` Krzysztof Kozlowski
  2013-12-17 14:58       ` Lee Jones
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2013-12-17 14:06 UTC (permalink / raw)
  To: Lee Jones
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Liam Girdwood,
	Mark Brown, Grant Likely, Sachin Kamat, Thomas Abraham,
	Amit Daniel Kachhap, devicetree, linux-doc, linux-kernel,
	Kyungmin Park, Marek Szyprowski

On Tue, 2013-12-17 at 13:51 +0000, Lee Jones wrote:
> > Add symbols for hard-coded values of BUCK_RAMP field in DVS_RAMP
> > register. This simplifies a little the code as register update is called
> > only once.
> > 
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Cc: Kyungmin Park <kyungmin.park@samsung.com>
> > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > ---
> >  drivers/regulator/s5m8767.c         |   22 ++++++++++------------
> >  include/linux/mfd/samsung/s5m8767.h |   15 +++++++++++++++
> >  2 files changed, 25 insertions(+), 12 deletions(-)
> 
> <snip>
> 
> > diff --git a/include/linux/mfd/samsung/s5m8767.h b/include/linux/mfd/samsung/s5m8767.h
> > index 9198377ee859..2ab0b0f03641 100644
> > --- a/include/linux/mfd/samsung/s5m8767.h
> > +++ b/include/linux/mfd/samsung/s5m8767.h
> > @@ -186,4 +186,19 @@ enum s5m8767_regulators {
> >  #define S5M8767_ENCTRL_SHIFT		6
> >  #define S5M8767_ENCTRL_MASK		(0x3 << S5M8767_ENCTRL_SHIFT)
> >  
> > +/*
> > + * Values for BUCK_RAMP field in DVS_RAMP register, matching raw values
> > + * in mV/us.
> > + */
> > +enum s5m8767_dvs_buck_ramp_values {
> > +	S5M8767_DVS_BUCK_RAMP_5		= 0x4,
> > +	S5M8767_DVS_BUCK_RAMP_10	= 0x9,
> > +	S5M8767_DVS_BUCK_RAMP_12_5	= 0xb,
> > +	S5M8767_DVS_BUCK_RAMP_25	= 0xd,
> > +	S5M8767_DVS_BUCK_RAMP_50	= 0xe,
> > +	S5M8767_DVS_BUCK_RAMP_100	= 0xf,
> > +};
> 
> Why do these have to be enums?

There is no special need. I chose enum because it is a kind of
enumeration of all valid values for BUCK_RAMP field (valid according to
datasheet). If you prefer #define then I'll change it.


Best regards,
Krzysztof



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

* Re: [PATCH 1/5] regulator: s5m8767: Add symbols for hard-coded DVS_RAMP register
  2013-12-17 14:06     ` Krzysztof Kozlowski
@ 2013-12-17 14:58       ` Lee Jones
  0 siblings, 0 replies; 19+ messages in thread
From: Lee Jones @ 2013-12-17 14:58 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Liam Girdwood,
	Mark Brown, Grant Likely, Sachin Kamat, Thomas Abraham,
	Amit Daniel Kachhap, devicetree, linux-doc, linux-kernel,
	Kyungmin Park, Marek Szyprowski

On Tue, 17 Dec 2013, Krzysztof Kozlowski wrote:

> On Tue, 2013-12-17 at 13:51 +0000, Lee Jones wrote:
> > > Add symbols for hard-coded values of BUCK_RAMP field in DVS_RAMP
> > > register. This simplifies a little the code as register update is called
> > > only once.
> > > 
> > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > > Cc: Kyungmin Park <kyungmin.park@samsung.com>
> > > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > > ---
> > >  drivers/regulator/s5m8767.c         |   22 ++++++++++------------
> > >  include/linux/mfd/samsung/s5m8767.h |   15 +++++++++++++++
> > >  2 files changed, 25 insertions(+), 12 deletions(-)
> > 
> > <snip>
> > 
> > > diff --git a/include/linux/mfd/samsung/s5m8767.h b/include/linux/mfd/samsung/s5m8767.h
> > > index 9198377ee859..2ab0b0f03641 100644
> > > --- a/include/linux/mfd/samsung/s5m8767.h
> > > +++ b/include/linux/mfd/samsung/s5m8767.h
> > > @@ -186,4 +186,19 @@ enum s5m8767_regulators {
> > >  #define S5M8767_ENCTRL_SHIFT		6
> > >  #define S5M8767_ENCTRL_MASK		(0x3 << S5M8767_ENCTRL_SHIFT)
> > >  
> > > +/*
> > > + * Values for BUCK_RAMP field in DVS_RAMP register, matching raw values
> > > + * in mV/us.
> > > + */
> > > +enum s5m8767_dvs_buck_ramp_values {
> > > +	S5M8767_DVS_BUCK_RAMP_5		= 0x4,
> > > +	S5M8767_DVS_BUCK_RAMP_10	= 0x9,
> > > +	S5M8767_DVS_BUCK_RAMP_12_5	= 0xb,
> > > +	S5M8767_DVS_BUCK_RAMP_25	= 0xd,
> > > +	S5M8767_DVS_BUCK_RAMP_50	= 0xe,
> > > +	S5M8767_DVS_BUCK_RAMP_100	= 0xf,
> > > +};
> > 
> > Why do these have to be enums?
> 
> There is no special need. I chose enum because it is a kind of
> enumeration of all valid values for BUCK_RAMP field (valid according to
> datasheet). If you prefer #define then I'll change it.

I'm not fussed enough to make you resubmit.

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 2/5] regulator: s5m8767: Do not use sec_reg* regmap helpers
  2013-12-17 13:23   ` [PATCH 2/5] regulator: s5m8767: Do not use sec_reg* regmap helpers Krzysztof Kozlowski
@ 2013-12-17 15:03     ` Lee Jones
       [not found]     ` <1387286601-21646-3-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
  1 sibling, 0 replies; 19+ messages in thread
From: Lee Jones @ 2013-12-17 15:03 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Liam Girdwood,
	Mark Brown, Grant Likely, Sachin Kamat, Thomas Abraham,
	Amit Daniel Kachhap, devicetree, linux-doc, linux-kernel,
	Kyungmin Park, Marek Szyprowski

On Tue, 17 Dec 2013, Krzysztof Kozlowski wrote:

> Replace calls to sec_reg* helpers with direct usage of regmap API. The
> sec_reg* helpers are error-prone as they mix u8 with unsigned int and
> order of some of parameters (val and mask in sec_reg_update()).
> 
> Also the helpers do not give any way of useful abstraction as they just
> call corresponding regmap function.
> 
> This patch replaces:
>  - sec_reg_read() with regmap_read(),
>  - sec_reg_write() with regmap_write(),
>  - sec_reg_update() with regmap_update_bits().
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/regulator/s5m8767.c |   71 +++++++++++++++++++++++++------------------
>  1 file changed, 41 insertions(+), 30 deletions(-)
> 

Mark,

As this is heavily reliant on the previous patch, feel free to
take it through the Regulator tree.

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 1/5] regulator: s5m8767: Add symbols for hard-coded DVS_RAMP register
  2013-12-17 13:23 ` [PATCH 1/5] regulator: s5m8767: Add symbols for hard-coded DVS_RAMP register Krzysztof Kozlowski
  2013-12-17 13:51   ` Lee Jones
@ 2013-12-18 13:27   ` Mark Brown
  2013-12-18 13:28   ` Mark Brown
  2 siblings, 0 replies; 19+ messages in thread
From: Mark Brown @ 2013-12-18 13:27 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Grant Likely, Sachin Kamat, Thomas Abraham, Amit Daniel Kachhap,
	devicetree, linux-doc, linux-kernel, Kyungmin Park,
	Marek Szyprowski

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

On Tue, Dec 17, 2013 at 02:23:17PM +0100, Krzysztof Kozlowski wrote:
> Add symbols for hard-coded values of BUCK_RAMP field in DVS_RAMP
> register. This simplifies a little the code as register update is called
> only once.

Applied, thanks.

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

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

* Re: [PATCH 1/5] regulator: s5m8767: Add symbols for hard-coded DVS_RAMP register
  2013-12-17 13:23 ` [PATCH 1/5] regulator: s5m8767: Add symbols for hard-coded DVS_RAMP register Krzysztof Kozlowski
  2013-12-17 13:51   ` Lee Jones
  2013-12-18 13:27   ` Mark Brown
@ 2013-12-18 13:28   ` Mark Brown
  2 siblings, 0 replies; 19+ messages in thread
From: Mark Brown @ 2013-12-18 13:28 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Grant Likely, Sachin Kamat, Thomas Abraham, Amit Daniel Kachhap,
	devicetree, linux-doc, linux-kernel, Kyungmin Park,
	Marek Szyprowski

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

On Tue, Dec 17, 2013 at 02:23:17PM +0100, Krzysztof Kozlowski wrote:
> Add symbols for hard-coded values of BUCK_RAMP field in DVS_RAMP
> register. This simplifies a little the code as register update is called
> only once.

Applied, thanks.

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

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

* Re: [PATCH 2/5] regulator: s5m8767: Do not use sec_reg* regmap helpers
       [not found]     ` <1387286601-21646-3-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@ 2013-12-18 13:28       ` Mark Brown
  0 siblings, 0 replies; 19+ messages in thread
From: Mark Brown @ 2013-12-18 13:28 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Grant Likely, Sachin Kamat, Thomas Abraham, Amit Daniel Kachhap,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Kyungmin Park,
	Marek Szyprowski

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

On Tue, Dec 17, 2013 at 02:23:18PM +0100, Krzysztof Kozlowski wrote:
> Replace calls to sec_reg* helpers with direct usage of regmap API. The
> sec_reg* helpers are error-prone as they mix u8 with unsigned int and
> order of some of parameters (val and mask in sec_reg_update()).

Applied, thanks.

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

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

* Re: [PATCH 3/5] mfd: sec: Remove sec_reg* regmap helpers
  2013-12-17 13:23 ` [PATCH 3/5] mfd: sec: Remove sec_reg* regmap helpers Krzysztof Kozlowski
@ 2013-12-18 13:36   ` Mark Brown
  2013-12-18 14:03     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2013-12-18 13:36 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Grant Likely, Sachin Kamat, Thomas Abraham, Amit Daniel Kachhap,
	devicetree, linux-doc, linux-kernel, Kyungmin Park,
	Marek Szyprowski

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

On Tue, Dec 17, 2013 at 02:23:19PM +0100, Krzysztof Kozlowski wrote:
> Remove sec_reg* helpers as they are not used anymore. These helpers were
> error-prone as they mixed u8 with unsigned int and they changed order of
> some of parameters (val and mask in sec_reg_update()).

This doesn't apply against the regulator tree.  Can you please check
what's going on here, I guess there may need to be some merge with MFD?

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

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

* Re: [PATCH 4/5] regulator: s5m8767: Use GPIO for controlling Buck9/eMMC
  2013-12-17 13:23 ` [PATCH 4/5] regulator: s5m8767: Use GPIO for controlling Buck9/eMMC Krzysztof Kozlowski
  2013-12-17 13:49   ` Lee Jones
@ 2013-12-18 13:47   ` Mark Brown
       [not found]     ` <20131218134720.GU28455-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  1 sibling, 1 reply; 19+ messages in thread
From: Mark Brown @ 2013-12-18 13:47 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Grant Likely, Sachin Kamat, Thomas Abraham, Amit Daniel Kachhap,
	devicetree, linux-doc, linux-kernel, Kyungmin Park,
	Marek Szyprowski

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

On Tue, Dec 17, 2013 at 02:23:20PM +0100, Krzysztof Kozlowski wrote:

> +static int s5m8767_reg_gpio_is_enabled(struct regulator_dev *rdev)
> +{
> +	struct s5m8767_info *s5m8767 = rdev_get_drvdata(rdev);
> +	int val;
> +
> +	if (!s5m8767->buck9_uses_gpio)
> +		return s5m8767_reg_is_enabled(rdev);
> +
> +	val = gpio_get_value(s5m8767->buck9_gpio);
> +
> +	return val == 1;
> +}

Don't open code this, use the core GPIO handling - see ena_gpio.

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

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

* Re: [PATCH 3/5] mfd: sec: Remove sec_reg* regmap helpers
  2013-12-18 13:36   ` Mark Brown
@ 2013-12-18 14:03     ` Krzysztof Kozlowski
  2013-12-18 17:25       ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2013-12-18 14:03 UTC (permalink / raw)
  To: Mark Brown
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Grant Likely, Sachin Kamat, Thomas Abraham, Amit Daniel Kachhap,
	devicetree, linux-doc, linux-kernel, Kyungmin Park,
	Marek Szyprowski

On Wed, 2013-12-18 at 13:36 +0000, Mark Brown wrote:
> On Tue, Dec 17, 2013 at 02:23:19PM +0100, Krzysztof Kozlowski wrote:
> > Remove sec_reg* helpers as they are not used anymore. These helpers were
> > error-prone as they mixed u8 with unsigned int and they changed order of
> > some of parameters (val and mask in sec_reg_update()).
> 
> This doesn't apply against the regulator tree.  Can you please check
> what's going on here, I guess there may need to be some merge with MFD?

I think you need to rebase on current Linus' (at least 3.13-rc4) to get
this patch:
"mfd/rtc: s5m: fix register updating by adding regmap for RTC" (3e1e4a5f3a3)

Best regards,
Krzysztof


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

* Re: [PATCH 4/5] regulator: s5m8767: Use GPIO for controlling Buck9/eMMC
       [not found]     ` <20131218134720.GU28455-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2013-12-18 14:57       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2013-12-18 14:57 UTC (permalink / raw)
  To: Mark Brown
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Grant Likely, Sachin Kamat, Thomas Abraham, Amit Daniel Kachhap,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Kyungmin Park,
	Marek Szyprowski

On Wed, 2013-12-18 at 13:47 +0000, Mark Brown wrote:
> On Tue, Dec 17, 2013 at 02:23:20PM +0100, Krzysztof Kozlowski wrote:
> 
> > +static int s5m8767_reg_gpio_is_enabled(struct regulator_dev *rdev)
> > +{
> > +	struct s5m8767_info *s5m8767 = rdev_get_drvdata(rdev);
> > +	int val;
> > +
> > +	if (!s5m8767->buck9_uses_gpio)
> > +		return s5m8767_reg_is_enabled(rdev);
> > +
> > +	val = gpio_get_value(s5m8767->buck9_gpio);
> > +
> > +	return val == 1;
> > +}
> 
> Don't open code this, use the core GPIO handling - see ena_gpio.

OK, I'll rewrite the patch.

Best regards,
Krzysztof

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 3/5] mfd: sec: Remove sec_reg* regmap helpers
  2013-12-18 14:03     ` Krzysztof Kozlowski
@ 2013-12-18 17:25       ` Mark Brown
  0 siblings, 0 replies; 19+ messages in thread
From: Mark Brown @ 2013-12-18 17:25 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, Sangbeom Kim, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Grant Likely, Sachin Kamat, Thomas Abraham, Amit Daniel Kachhap,
	devicetree, linux-doc, linux-kernel, Kyungmin Park,
	Marek Szyprowski

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

On Wed, Dec 18, 2013 at 03:03:51PM +0100, Krzysztof Kozlowski wrote:
> On Wed, 2013-12-18 at 13:36 +0000, Mark Brown wrote:

> > This doesn't apply against the regulator tree.  Can you please check
> > what's going on here, I guess there may need to be some merge with MFD?

> I think you need to rebase on current Linus' (at least 3.13-rc4) to get
> this patch:
> "mfd/rtc: s5m: fix register updating by adding regmap for RTC" (3e1e4a5f3a3)

Yes, that's it - applied now, thanks.

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

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

end of thread, other threads:[~2013-12-18 17:25 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-17 13:23 [PATCH 0/5] mfd/regulator: s5m: Various fixes and GPIO control over Buck9 Krzysztof Kozlowski
2013-12-17 13:23 ` [PATCH 1/5] regulator: s5m8767: Add symbols for hard-coded DVS_RAMP register Krzysztof Kozlowski
2013-12-17 13:51   ` Lee Jones
2013-12-17 14:06     ` Krzysztof Kozlowski
2013-12-17 14:58       ` Lee Jones
2013-12-18 13:27   ` Mark Brown
2013-12-18 13:28   ` Mark Brown
     [not found] ` <1387286601-21646-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2013-12-17 13:23   ` [PATCH 2/5] regulator: s5m8767: Do not use sec_reg* regmap helpers Krzysztof Kozlowski
2013-12-17 15:03     ` Lee Jones
     [not found]     ` <1387286601-21646-3-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2013-12-18 13:28       ` Mark Brown
2013-12-17 13:23   ` [PATCH 5/5] regulator: s5m8767: Document new bindings for Buck9 GPIO control Krzysztof Kozlowski
2013-12-17 13:23 ` [PATCH 3/5] mfd: sec: Remove sec_reg* regmap helpers Krzysztof Kozlowski
2013-12-18 13:36   ` Mark Brown
2013-12-18 14:03     ` Krzysztof Kozlowski
2013-12-18 17:25       ` Mark Brown
2013-12-17 13:23 ` [PATCH 4/5] regulator: s5m8767: Use GPIO for controlling Buck9/eMMC Krzysztof Kozlowski
2013-12-17 13:49   ` Lee Jones
2013-12-18 13:47   ` Mark Brown
     [not found]     ` <20131218134720.GU28455-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-12-18 14:57       ` Krzysztof Kozlowski

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