[PATCH v2 0/2] regulator: s5m8767: Use GPIO for controlling Buck9/eMMC

[PATCH v2 0/2] regulator: s5m8767: Use GPIO for controlling Buck9/eMMC

[Krzysztof Kozlowski]

Hi,

This is second version of patch adding GPIO control over Buck9 regulator
in s5m8767 regulator driver.

Previously the patch was part of larger patchset from which all other patches
were applied. Changes since previous revision:
1. Use ena_gpio of regulator core, as suggested by Mark Brown.

This patchset is based on next tree: next-20140122.

Best regards,
Krzysztof

Krzysztof Kozlowski (2):
  regulator: s5m8767: Use GPIO for controlling Buck9/eMMC
  regulator: s5m8767: Document new bindings for Buck9 GPIO control

 .../bindings/regulator/s5m8767-regulator.txt       |   16 +++-
 drivers/regulator/s5m8767.c                        |   97 +++++++++++++++++++-
 include/linux/mfd/samsung/core.h                   |    4 +-
 include/linux/mfd/samsung/s5m8767.h                |    7 ++
 4 files changed, 119 insertions(+), 5 deletions(-)

-- 
1.7.9.5

[PATCH v2 1/2] regulator: s5m8767: Use GPIO for controlling Buck9/eMMC

[Krzysztof Kozlowski]

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>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/regulator/s5m8767.c         |  101 +++++++++++++++++++++++++++++++++--
 include/linux/mfd/samsung/core.h    |    4 +-
 include/linux/mfd/samsung/s5m8767.h |    7 +++
 3 files changed, 108 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
index d7164bb75d3e..95c28b04dcc9 100644
--- a/drivers/regulator/s5m8767.c
+++ b/drivers/regulator/s5m8767.c
@@ -11,11 +11,8 @@
  *
  */
 
-#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>
 #include <linux/regulator/driver.h>
@@ -483,6 +480,65 @@ static struct regulator_desc regulators[] = {
 	s5m8767_regulator_desc(BUCK9),
 };
 
+/*
+ * Enable GPIO control over BUCK9 in regulator_config for that regulator.
+ */
+static void s5m8767_regulator_config_ext_control(struct s5m8767_info *s5m8767,
+		struct sec_regulator_data *rdata,
+		struct regulator_config *config)
+{
+	int i, mode = 0;
+
+	if (rdata->id != S5M8767_BUCK9)
+		return;
+
+	/* Check if opmode for regulator matches S5M8767_ENCTRL_USE_GPIO */
+	for (i = 0; i < s5m8767->num_regulators; i++) {
+		const struct sec_opmode_data *opmode = &s5m8767->opmode[i];
+		if (opmode->id == rdata->id) {
+			mode = s5m8767_opmode_reg[rdata->id][opmode->mode];
+			break;
+		}
+	}
+	if (mode != S5M8767_ENCTRL_USE_GPIO) {
+		dev_warn(s5m8767->dev,
+				"ext-control for %s: mismatched op_mode (%x), ignoring\n",
+				rdata->reg_node->name, mode);
+		return;
+	}
+
+	if (!gpio_is_valid(rdata->ext_control_gpio)) {
+		dev_warn(s5m8767->dev,
+				"ext-control for %s: GPIO not valid, ignoring\n",
+				rdata->reg_node->name);
+		return;
+	}
+
+	config->ena_gpio = rdata->ext_control_gpio;
+	config->ena_gpio_flags = GPIOF_OUT_INIT_HIGH;
+}
+
+/*
+ * Turn on GPIO control over BUCK9.
+ */
+static int s5m8767_enable_ext_control(struct s5m8767_info *s5m8767,
+		struct regulator_dev *rdev)
+{
+	int ret, reg, enable_ctrl;
+
+	if (rdev_get_id(rdev) != S5M8767_BUCK9)
+		return -EINVAL;
+
+	ret = s5m8767_get_register(rdev, &reg, &enable_ctrl);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(s5m8767->iodev->regmap_pmic,
+			reg, S5M8767_ENCTRL_MASK,
+			S5M8767_ENCTRL_USE_GPIO << S5M8767_ENCTRL_SHIFT);
+}
+
+
 #ifdef CONFIG_OF
 static int s5m8767_pmic_dt_parse_dvs_gpio(struct sec_pmic_dev *iodev,
 			struct sec_platform_data *pdata,
@@ -520,6 +576,24 @@ static int s5m8767_pmic_dt_parse_ds_gpio(struct sec_pmic_dev *iodev,
 	return 0;
 }
 
+static int s5m8767_pmic_dt_parse_ext_control_gpio(struct sec_pmic_dev *iodev,
+		struct sec_regulator_data *rdata,
+		struct device_node *reg_np)
+{
+	if (of_get_property(reg_np, "s5m8767,pmic-ext-control-enable", NULL)) {
+		int gpio = of_get_named_gpio(reg_np,
+					"s5m8767,pmic-ext-control-gpio", 0);
+		if (!gpio_is_valid(gpio)) {
+			dev_err(iodev->dev, "invalid %s gpio: %d\n",
+					reg_np->name, gpio);
+			return -EINVAL;
+		}
+		rdata->ext_control_enable = true;
+		rdata->ext_control_gpio = gpio;
+	}
+	return 0;
+}
+
 static int s5m8767_pmic_dt_parse_pdata(struct platform_device *pdev,
 					struct sec_platform_data *pdata)
 {
@@ -574,6 +648,14 @@ static int s5m8767_pmic_dt_parse_pdata(struct platform_device *pdev,
 			continue;
 		}
 
+		if (s5m8767_pmic_dt_parse_ext_control_gpio(iodev, rdata,
+					reg_np)) {
+			dev_warn(iodev->dev,
+				"wrong configuration for regulator %s, skipping\n",
+				reg_np->name);
+			continue;
+		}
+
 		rdata->id = i;
 		rdata->initdata = of_get_regulator_init_data(
 						&pdev->dev, reg_np);
@@ -940,6 +1022,9 @@ static int s5m8767_pmic_probe(struct platform_device *pdev)
 		config.driver_data = s5m8767;
 		config.regmap = iodev->regmap_pmic;
 		config.of_node = pdata->regulators[i].reg_node;
+		if (pdata->regulators[i].ext_control_enable)
+			s5m8767_regulator_config_ext_control(s5m8767,
+					&pdata->regulators[i], &config);
 
 		rdev[i] = devm_regulator_register(&pdev->dev, &regulators[id],
 						  &config);
@@ -949,6 +1034,16 @@ static int s5m8767_pmic_probe(struct platform_device *pdev)
 					id);
 			return ret;
 		}
+
+		if (pdata->regulators[i].ext_control_enable) {
+			ret = s5m8767_enable_ext_control(s5m8767, rdev[i]);
+			if (ret < 0) {
+				dev_err(s5m8767->dev,
+						"failed to enable gpio control over %s: %d\n",
+						rdev[i]->desc->name, ret);
+				return ret;
+			}
+		}
 	}
 
 	return 0;
diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
index 41c9bde410c5..867f2036ce17 100644
--- a/include/linux/mfd/samsung/core.h
+++ b/include/linux/mfd/samsung/core.h
@@ -119,7 +119,9 @@ struct sec_platform_data {
 struct sec_regulator_data {
 	int				id;
 	struct regulator_init_data	*initdata;
-	struct device_node *reg_node;
+	struct device_node		*reg_node;
+	int				ext_control_gpio;
+	bool				ext_control_enable;
 };
 
 /*
diff --git a/include/linux/mfd/samsung/s5m8767.h b/include/linux/mfd/samsung/s5m8767.h
index 2ab0b0f03641..243b58fec33d 100644
--- a/include/linux/mfd/samsung/s5m8767.h
+++ b/include/linux/mfd/samsung/s5m8767.h
@@ -183,10 +183,17 @@ 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
  * in mV/us.
  */
-- 
1.7.9.5

[PATCH v2 2/2] regulator: s5m8767: Document new bindings for Buck9 GPIO control

[Krzysztof Kozlowski]

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

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 .../bindings/regulator/s5m8767-regulator.txt       |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt b/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
index fc6b38f035bd..b52ee77dc4c0 100644
--- a/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
@@ -69,13 +69,18 @@ sub-node should be of the format as listed below.
 		};
 	};
 The above regulator entries are defined in regulator bindings documentation
-except op_mode description.
+except these properties:
 	- op_mode: describes the different operating modes of the LDO's with
 		power mode change in SOC. The different possible values are,
 		0 - always off mode
 		1 - on in normal mode
 		2 - low power mode
 		3 - suspend mode
+	- s5m8767,pmic-ext-control-enable: regulator can be enabled/disabled
+		by GPIO (valid only for buck9).
+	- s5m8767,pmic-ext-control-gpio: GPIO specifier for one GPIO
+		controlling this regulator (valid only for buck9).
+		This property is required when 's5m8767,pmic-ext-control-enable' is specified.
 
 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
@@ -148,5 +153,14 @@ 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 */
+				s5m8767,pmic-ext-control-enable;
+				s5m8767,pmic-ext-control-gpio = <&gpk0 2 0>;
+			};
 		};
 	};
-- 
1.7.9.5

Re: [PATCH v2 2/2] regulator: s5m8767: Document new bindings for Buck9 GPIO control

[Mark Brown]

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

On Wed, Jan 22, 2014 at 05:07:28PM +0100, Krzysztof Kozlowski wrote:
> Add documentation for new bindings for controlling (enable/disable) the
> Buck9 Converter by GPIO (BUCK9EN).

Your CC list for this is *very* large...

> +	- s5m8767,pmic-ext-control-enable: regulator can be enabled/disabled
> +		by GPIO (valid only for buck9).
> +	- s5m8767,pmic-ext-control-gpio: GPIO specifier for one GPIO
> +		controlling this regulator (valid only for buck9).
> +		This property is required when 's5m8767,pmic-ext-control-enable' is specified.

In what situation might the GPIO be present but not usable - can't we
just use the presence of the GPIO property?  Also GPIO properties are
supposed to be always "-gpios".

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

Re: [PATCH v2 2/2] regulator: s5m8767: Document new bindings for Buck9 GPIO control

[Krzysztof Kozlowski]

On Wed, 2014-01-22 at 19:49 +0000, Mark Brown wrote:
> On Wed, Jan 22, 2014 at 05:07:28PM +0100, Krzysztof Kozlowski wrote:
> > Add documentation for new bindings for controlling (enable/disable) the
> > Buck9 Converter by GPIO (BUCK9EN).
> 
> Your CC list for this is *very* large...
Hmmm... The get_maintainers produces such long list for any change in
Documentation/devicetree/bindings... I'll stop using it for this.

> 
> > +	- s5m8767,pmic-ext-control-enable: regulator can be enabled/disabled
> > +		by GPIO (valid only for buck9).
> > +	- s5m8767,pmic-ext-control-gpio: GPIO specifier for one GPIO
> > +		controlling this regulator (valid only for buck9).
> > +		This property is required when 's5m8767,pmic-ext-control-enable' is specified.
> 
> In what situation might the GPIO be present but not usable - can't we
> just use the presence of the GPIO property?  Also GPIO properties are
> supposed to be always "-gpios".
Remove the "s5m8767,pmic-ext-control-enable" and use only
"s5m8767,pmic-ext-control-gpios"? Sure, that makes sense. Thanks for
idea.


Best regards,
Krzysztof