[PATCH v5 0/5] regulator: Allow parsing custom DT properties with simplified DT parse

[PATCH v5 0/5] regulator: Allow parsing custom DT properties with simplified DT parse

[Krzysztof Kozlowski]

Hi,


The patchset adds:
1. a way of parsing custom DT properties by the driver when simplified
   DT parsing method is used,
2. GPIO enable control to the max77686 driver.


Rationale
=========
After converting drivers to simplified DT parsing method, the parsing
is done by regulator core. The driver cannot longer parse any custom
properties from Device Tree. Example of such special property is
GPIO for enable control.

Driver may want to rely on regulator core to parse most of the bindings
but still need to parse one custom stuff. Additionally driver may need
to perform some special activities depending on Device Tree.

This is solved by adding a callback called from the core when regulator
is matched from DT. The max77686 driver uses it for:
1. parsing "maxim,ena-gpios" property from regulator OF node,
2. enabling GPIO control for such regulator.


Changes since v4
================
1. Re-work idea. Give up generic ena-gpios binding and let driver
   parse any custom property.

Changes since v3
================
1. Regulator cleanup patches were applied by Mark, drop them.
2. Re-work idea by adding generic ena-gpios binding.

Changes since v2
================
Re-work the board file support removal after Javier's comments: use
new DT style parsing. This imposes a lot of changes.
1. Add "of_compatible" for regulator drivers.
2. Provide backward compatibility if such "of_compatible" is not present.
   The driver will search for regulators node and use it as dev->of_node.
   Everything should be bisect-friendly.
3. New patches: 1, 2, 3, 5, 13 and 14.
4. Because of new style DT parsing it is much easier to put "gpio"
   properties in regulators top node (not in each regulator).
   This will be also new-gpio-lib friendly.

Changes since v1
================
1. Add patch: 1/8 "regulator: max77686: Consistently index opmode
   array by rdev id".
2. Remove patch "regulator: max77686: Make regulator_desc array
   const" (applied).
3. Re-work patches removing from regulators board file support (2/8
   and 3/8). Parse regulators with of_regulator_match() at once, remove
   num_regulators.
4. Patch 4/8: Add depends on OF to mfd/Kconfig. Add Javier's
   reviewed-by.
5. Patch 5/8: Add Javier's reviewed-by.
6. Patch 6/8: Add depends on GPIOLIB to regulator/Kconfig. Rename
   "external control" to "GPIO control" and "ext_control_gpios" to
   simpler "gpios".
   I tried to use new GPIO API but it ended with more problems
   https://lkml.org/lkml/2014/10/29/239


Best regards,
Krzysztof


Krzysztof Kozlowski (5):
  regulator: Copy config passed during registration
  regulator: Allow parsing custom properties when using simplified DT
    parsing
  regulator: max77686: Add GPIO control
  mfd/regulator: dt-bindings: max77686: Document gpio properties
  ARM: dts: exynos4412-trats: Switch max77686 regulators to GPIO control

 Documentation/devicetree/bindings/mfd/max77686.txt | 14 +++++
 arch/arm/boot/dts/exynos4412-trats2.dts            | 25 ++------
 drivers/regulator/core.c                           | 20 +++++--
 drivers/regulator/internal.h                       |  2 +
 drivers/regulator/max77686.c                       | 70 ++++++++++++++++++++--
 drivers/regulator/of_regulator.c                   | 11 ++++
 include/linux/regulator/driver.h                   | 13 ++++
 7 files changed, 126 insertions(+), 29 deletions(-)

-- 
1.9.1

[PATCH v5 1/5] regulator: Copy config passed during registration

[Krzysztof Kozlowski]

Copy the 'regulator_config' structure passed to regulator_register()
function so the driver could safely modify it after parsing init data.

The driver may want to change the config as a result of specific init
data parsed by regulator core (e.g. when core handled parsing device
tree).

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/regulator/core.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e225711bb8bc..c13b557a560e 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3581,20 +3581,21 @@ static void rdev_init_debugfs(struct regulator_dev *rdev)
  */
 struct regulator_dev *
 regulator_register(const struct regulator_desc *regulator_desc,
-		   const struct regulator_config *config)
+		   const struct regulator_config *cfg)
 {
 	const struct regulation_constraints *constraints = NULL;
 	const struct regulator_init_data *init_data;
+	struct regulator_config *config = NULL;
 	static atomic_t regulator_no = ATOMIC_INIT(0);
 	struct regulator_dev *rdev;
 	struct device *dev;
 	int ret, i;
 	const char *supply = NULL;
 
-	if (regulator_desc == NULL || config == NULL)
+	if (regulator_desc == NULL || cfg == NULL)
 		return ERR_PTR(-EINVAL);
 
-	dev = config->dev;
+	dev = cfg->dev;
 	WARN_ON(!dev);
 
 	if (regulator_desc->name == NULL || regulator_desc->ops == NULL)
@@ -3624,6 +3625,16 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	if (rdev == NULL)
 		return ERR_PTR(-ENOMEM);
 
+	/*
+	 * Duplicate the config so the driver could override it after
+	 * parsing init data.
+	 */
+	config = kmemdup(cfg, sizeof(*cfg), GFP_KERNEL);
+	if (config == NULL) {
+		kfree(rdev);
+		return ERR_PTR(-ENOMEM);
+	}
+
 	init_data = regulator_of_get_init_data(dev, regulator_desc,
 					       &rdev->dev.of_node);
 	if (!init_data) {
@@ -3752,6 +3763,7 @@ add_dev:
 	rdev_init_debugfs(rdev);
 out:
 	mutex_unlock(&regulator_list_mutex);
+	kfree(config);
 	return rdev;
 
 unset_supplies:
-- 
1.9.1

[PATCH v5 2/5] regulator: Allow parsing custom properties when using simplified DT parsing

[Krzysztof Kozlowski]

When drivers use simplified DT parsing method (they provide
'regulator_desc.of_match') they still may want to parse custom
properties for some of the regulators. For example some of the
regulators support GPIO enable control.

Add a driver-supplied callback for such case. This way the regulator
core parses common bindings offloading a lot of code from drivers and
still custom properties may be used.

The callback, called for each parsed regulator, may modify the
'regulator_config' initially passed to regulator_register().

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/regulator/core.c         |  2 +-
 drivers/regulator/internal.h     |  2 ++
 drivers/regulator/of_regulator.c | 11 +++++++++++
 include/linux/regulator/driver.h | 13 +++++++++++++
 4 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index c13b557a560e..5fae8cabd254 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3635,7 +3635,7 @@ regulator_register(const struct regulator_desc *regulator_desc,
 		return ERR_PTR(-ENOMEM);
 	}
 
-	init_data = regulator_of_get_init_data(dev, regulator_desc,
+	init_data = regulator_of_get_init_data(dev, regulator_desc, config,
 					       &rdev->dev.of_node);
 	if (!init_data) {
 		init_data = config->init_data;
diff --git a/drivers/regulator/internal.h b/drivers/regulator/internal.h
index 80ba2a35a04b..c74ac8734023 100644
--- a/drivers/regulator/internal.h
+++ b/drivers/regulator/internal.h
@@ -38,11 +38,13 @@ struct regulator {
 #ifdef CONFIG_OF
 struct regulator_init_data *regulator_of_get_init_data(struct device *dev,
 			         const struct regulator_desc *desc,
+				 struct regulator_config *config,
 				 struct device_node **node);
 #else
 static inline struct regulator_init_data *
 regulator_of_get_init_data(struct device *dev,
 			   const struct regulator_desc *desc,
+			   struct regulator_config *config,
 			   struct device_node **node)
 {
 	return NULL;
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 91eaaf010524..24e812c48d93 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -270,6 +270,7 @@ EXPORT_SYMBOL_GPL(of_regulator_match);
 
 struct regulator_init_data *regulator_of_get_init_data(struct device *dev,
 					    const struct regulator_desc *desc,
+					    struct regulator_config *config,
 					    struct device_node **node)
 {
 	struct device_node *search, *child;
@@ -307,6 +308,16 @@ struct regulator_init_data *regulator_of_get_init_data(struct device *dev,
 			break;
 		}
 
+		if (desc->of_parse_cb) {
+			if (desc->of_parse_cb(child, desc, config)) {
+				dev_err(dev,
+					"driver callback failed to parse DT for regulator %s\n",
+					child->name);
+				init_data = NULL;
+				break;
+			}
+		}
+
 		of_node_get(child);
 		*node = child;
 		break;
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 5f1e9ca47417..d4ad5b5a02bb 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -21,6 +21,7 @@
 
 struct regmap;
 struct regulator_dev;
+struct regulator_config;
 struct regulator_init_data;
 struct regulator_enable_gpio;
 
@@ -205,6 +206,15 @@ enum regulator_type {
  * @supply_name: Identifying the regulator supply
  * @of_match: Name used to identify regulator in DT.
  * @regulators_node: Name of node containing regulator definitions in DT.
+ * @of_parse_cb: Optional callback called only if of_match is present.
+ *               Will be called for each regulator parsed from DT, during
+ *               init_data parsing.
+ *               The regulator_config passed as argument to the callback will
+ *               be a copy of config passed to regulator_register, valid only
+ *               for this particular call. Callback may freely change the
+ *               config but it cannot store it for later usage.
+ *               Callback should return 0 on success or negative ERRNO
+ *               indicating failure.
  * @id: Numerical identifier for the regulator.
  * @ops: Regulator operations table.
  * @irq: Interrupt number for the regulator.
@@ -251,6 +261,9 @@ struct regulator_desc {
 	const char *supply_name;
 	const char *of_match;
 	const char *regulators_node;
+	int (*of_parse_cb)(struct device_node *,
+			    const struct regulator_desc *,
+			    struct regulator_config *);
 	int id;
 	bool continuous_voltage_range;
 	unsigned n_voltages;
-- 
1.9.1

[PATCH v5 3/5] regulator: max77686: Add GPIO control

[Krzysztof Kozlowski]

Add enable control over GPIO for regulators supporting this: LDO20,
LDO21, LDO22, buck8 and buck9.

This is needed for proper (and full) configuration of the Maxim 77686
PMIC without creating redundant 'regulator-fixed' entries.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/regulator/max77686.c | 70 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 65 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
index 10d206266ac2..15fb1416bfbd 100644
--- a/drivers/regulator/max77686.c
+++ b/drivers/regulator/max77686.c
@@ -26,6 +26,7 @@
 #include <linux/bug.h>
 #include <linux/err.h>
 #include <linux/gpio.h>
+#include <linux/of_gpio.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/driver.h>
@@ -46,6 +47,11 @@
 #define MAX77686_DVS_UVSTEP	12500
 
 /*
+ * Value for configuring buck[89] and LDO{20,21,22} as GPIO control.
+ * It is the same as 'off' for other regulators.
+ */
+#define MAX77686_GPIO_CONTROL		0x0
+/*
  * Values used for configuring LDOs and bucks.
  * Forcing low power mode: LDO1, 3-5, 9, 13, 17-26
  */
@@ -82,6 +88,8 @@ enum max77686_ramp_rate {
 };
 
 struct max77686_data {
+	u64 gpio_enabled:MAX77686_REGULATORS;
+
 	/* Array indexed by regulator id */
 	unsigned int opmode[MAX77686_REGULATORS];
 };
@@ -100,6 +108,26 @@ static unsigned int max77686_get_opmode_shift(int id)
 	}
 }
 
+/*
+ * When regulator is configured for GPIO control then it
+ * replaces "normal" mode. Any change from low power mode to normal
+ * should actually change to GPIO control.
+ * Map normal mode to proper value for such regulators.
+ */
+static unsigned int max77686_map_normal_mode(struct max77686_data *max77686,
+					     int id)
+{
+	switch (id) {
+	case MAX77686_BUCK8:
+	case MAX77686_BUCK9:
+	case MAX77686_LDO20 ... MAX77686_LDO22:
+		if (max77686->gpio_enabled & (1 << id))
+			return MAX77686_GPIO_CONTROL;
+	}
+
+	return MAX77686_NORMAL;
+}
+
 /* Some BUCKs and LDOs supports Normal[ON/OFF] mode during suspend */
 static int max77686_set_suspend_disable(struct regulator_dev *rdev)
 {
@@ -136,7 +164,7 @@ static int max77686_set_suspend_mode(struct regulator_dev *rdev,
 		val = MAX77686_LDO_LOWPOWER_PWRREQ;
 		break;
 	case REGULATOR_MODE_NORMAL:			/* ON in Normal Mode */
-		val = MAX77686_NORMAL;
+		val = max77686_map_normal_mode(max77686, id);
 		break;
 	default:
 		pr_warn("%s: regulator_suspend_mode : 0x%x not supported\n",
@@ -160,7 +188,7 @@ static int max77686_ldo_set_suspend_mode(struct regulator_dev *rdev,
 {
 	unsigned int val;
 	struct max77686_data *max77686 = rdev_get_drvdata(rdev);
-	int ret;
+	int ret, id = rdev_get_id(rdev);
 
 	switch (mode) {
 	case REGULATOR_MODE_STANDBY:			/* switch off */
@@ -170,7 +198,7 @@ static int max77686_ldo_set_suspend_mode(struct regulator_dev *rdev,
 		val = MAX77686_LDO_LOWPOWER_PWRREQ;
 		break;
 	case REGULATOR_MODE_NORMAL:			/* ON in Normal Mode */
-		val = MAX77686_NORMAL;
+		val = max77686_map_normal_mode(max77686, id);
 		break;
 	default:
 		pr_warn("%s: regulator_suspend_mode : 0x%x not supported\n",
@@ -184,7 +212,7 @@ static int max77686_ldo_set_suspend_mode(struct regulator_dev *rdev,
 	if (ret)
 		return ret;
 
-	max77686->opmode[rdev_get_id(rdev)] = val;
+	max77686->opmode[id] = val;
 	return 0;
 }
 
@@ -197,7 +225,7 @@ static int max77686_enable(struct regulator_dev *rdev)
 	shift = max77686_get_opmode_shift(id);
 
 	if (max77686->opmode[id] == MAX77686_OFF_PWRREQ)
-		max77686->opmode[id] = MAX77686_NORMAL;
+		max77686->opmode[id] = max77686_map_normal_mode(max77686, id);
 
 	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
 				  rdev->desc->enable_mask,
@@ -229,6 +257,36 @@ static int max77686_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
 				  MAX77686_RAMP_RATE_MASK, ramp_value << 6);
 }
 
+static int max77686_of_parse_cb(struct device_node *np,
+		const struct regulator_desc *desc,
+		struct regulator_config *config)
+{
+	struct max77686_data *max77686 = config->driver_data;
+
+	switch (desc->id) {
+	case MAX77686_BUCK8:
+	case MAX77686_BUCK9:
+	case MAX77686_LDO20 ... MAX77686_LDO22:
+		config->ena_gpio = of_get_named_gpio(np,
+					"maxim,ena-gpios", 0);
+		config->ena_gpio_flags = GPIOF_OUT_INIT_HIGH;
+		config->ena_gpio_initialized = true;
+		break;
+	default:
+		return 0;
+	}
+
+	if (gpio_is_valid(config->ena_gpio)) {
+		max77686->gpio_enabled |= (1 << desc->id);
+
+		return regmap_update_bits(config->regmap, desc->enable_reg,
+					  desc->enable_mask,
+					  MAX77686_GPIO_CONTROL);
+	}
+
+	return 0;
+}
+
 static struct regulator_ops max77686_ops = {
 	.list_voltage		= regulator_list_voltage_linear,
 	.map_voltage		= regulator_map_voltage_linear,
@@ -283,6 +341,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
 	.name		= "LDO"#num,					\
 	.of_match	= of_match_ptr("LDO"#num),			\
 	.regulators_node	= of_match_ptr("voltage-regulators"),	\
+	.of_parse_cb	= max77686_of_parse_cb,				\
 	.id		= MAX77686_LDO##num,				\
 	.ops		= &max77686_ops,				\
 	.type		= REGULATOR_VOLTAGE,				\
@@ -355,6 +414,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
 	.name		= "BUCK"#num,					\
 	.of_match	= of_match_ptr("BUCK"#num),			\
 	.regulators_node	= of_match_ptr("voltage-regulators"),	\
+	.of_parse_cb	= max77686_of_parse_cb,				\
 	.id		= MAX77686_BUCK##num,				\
 	.ops		= &max77686_ops,				\
 	.type		= REGULATOR_VOLTAGE,				\
-- 
1.9.1

[PATCH v5 4/5] mfd/regulator: dt-bindings: max77686: Document gpio properties

[Krzysztof Kozlowski]

Document usage of maxim,ena-gpios properties which turn on external/GPIO
control over regulator.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 Documentation/devicetree/bindings/mfd/max77686.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt b/Documentation/devicetree/bindings/mfd/max77686.txt
index 75fdfaf41831..e39f0bc1f55e 100644
--- a/Documentation/devicetree/bindings/mfd/max77686.txt
+++ b/Documentation/devicetree/bindings/mfd/max77686.txt
@@ -39,6 +39,12 @@ to get matched with their hardware counterparts as follow:
 	-BUCKn	:	1-4.
   Use standard regulator bindings for it ('regulator-off-in-suspend').
 
+  LDO20, LDO21, LDO22, BUCK8 and BUCK9 can be configured to GPIO enable
+  control. To turn this feature on this property must be added to the regulator
+  sub-node:
+	- maxim,ena-gpios :	one GPIO specifier enable control (the gpio
+				flags are actually ignored and always
+				ACTIVE_HIGH is used)
 
 Example:
 
@@ -65,4 +71,12 @@ Example:
 				regulator-always-on;
 				regulator-boot-on;
 			};
+
+			buck9_reg {
+				regulator-compatible = "BUCK9";
+				regulator-name = "CAM_ISP_CORE_1.2V";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1200000>;
+				maxim,ena-gpios = <&gpm0 3 GPIO_ACTIVE_HIGH>;
+			};
 	}
-- 
1.9.1

[PATCH v5 4/5] mfd/regulator: dt-bindings: max77686: Document gpio properties

[Lee Jones]

On Mon, 05 Jan 2015, Krzysztof Kozlowski wrote:

> Document usage of maxim,ena-gpios properties which turn on external/GPIO
> control over regulator.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>  Documentation/devicetree/bindings/mfd/max77686.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt b/Documentation/devicetree/bindings/mfd/max77686.txt
> index 75fdfaf41831..e39f0bc1f55e 100644
> --- a/Documentation/devicetree/bindings/mfd/max77686.txt
> +++ b/Documentation/devicetree/bindings/mfd/max77686.txt
> @@ -39,6 +39,12 @@ to get matched with their hardware counterparts as follow:
>  	-BUCKn	:	1-4.
>    Use standard regulator bindings for it ('regulator-off-in-suspend').
>  
> +  LDO20, LDO21, LDO22, BUCK8 and BUCK9 can be configured to GPIO enable
> +  control. To turn this feature on this property must be added to the regulator
> +  sub-node:
> +	- maxim,ena-gpios :	one GPIO specifier enable control (the gpio
> +				flags are actually ignored and always
> +				ACTIVE_HIGH is used)

How does this differ to the 'enable-gpio' property which has already
been defined?

>  Example:
>  
> @@ -65,4 +71,12 @@ Example:
>  				regulator-always-on;
>  				regulator-boot-on;
>  			};
> +
> +			buck9_reg {
> +				regulator-compatible = "BUCK9";
> +				regulator-name = "CAM_ISP_CORE_1.2V";
> +				regulator-min-microvolt = <1000000>;
> +				regulator-max-microvolt = <1200000>;
> +				maxim,ena-gpios = <&gpm0 3 GPIO_ACTIVE_HIGH>;
> +			};
>  	}

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

[PATCH v5 4/5] mfd/regulator: dt-bindings: max77686: Document gpio properties

[Krzysztof Kozlowski]

On wto, 2015-01-20 at 13:41 +0000, Lee Jones wrote:
> On Mon, 05 Jan 2015, Krzysztof Kozlowski wrote:
> 
> > Document usage of maxim,ena-gpios properties which turn on external/GPIO
> > control over regulator.
> > 
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > ---
> >  Documentation/devicetree/bindings/mfd/max77686.txt | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt b/Documentation/devicetree/bindings/mfd/max77686.txt
> > index 75fdfaf41831..e39f0bc1f55e 100644
> > --- a/Documentation/devicetree/bindings/mfd/max77686.txt
> > +++ b/Documentation/devicetree/bindings/mfd/max77686.txt
> > @@ -39,6 +39,12 @@ to get matched with their hardware counterparts as follow:
> >  	-BUCKn	:	1-4.
> >    Use standard regulator bindings for it ('regulator-off-in-suspend').
> >  
> > +  LDO20, LDO21, LDO22, BUCK8 and BUCK9 can be configured to GPIO enable
> > +  control. To turn this feature on this property must be added to the regulator
> > +  sub-node:
> > +	- maxim,ena-gpios :	one GPIO specifier enable control (the gpio
> > +				flags are actually ignored and always
> > +				ACTIVE_HIGH is used)
> 
> How does this differ to the 'enable-gpio' property which has already
> been defined?

Strictly speaking - no difference, just a GPIO specifier.

However the 'enable-gpio' property exists only for gpio-regulator. The
regulator core does not support generic enable-gpio property and each
driver implements it on its own.

Anyway Mark Brown already applied this patch to his regulator tree.

Best regards,
Krzysztof

> 
> >  Example:
> >  
> > @@ -65,4 +71,12 @@ Example:
> >  				regulator-always-on;
> >  				regulator-boot-on;
> >  			};
> > +
> > +			buck9_reg {
> > +				regulator-compatible = "BUCK9";
> > +				regulator-name = "CAM_ISP_CORE_1.2V";
> > +				regulator-min-microvolt = <1000000>;
> > +				regulator-max-microvolt = <1200000>;
> > +				maxim,ena-gpios = <&gpm0 3 GPIO_ACTIVE_HIGH>;
> > +			};
> >  	}
> 

[PATCH v5 4/5] mfd/regulator: dt-bindings: max77686: Document gpio properties

[Lee Jones]

On Tue, 20 Jan 2015, Krzysztof Kozlowski wrote:

> On wto, 2015-01-20 at 13:41 +0000, Lee Jones wrote:
> > On Mon, 05 Jan 2015, Krzysztof Kozlowski wrote:
> > 
> > > Document usage of maxim,ena-gpios properties which turn on external/GPIO
> > > control over regulator.
> > > 
> > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > > ---
> > >  Documentation/devicetree/bindings/mfd/max77686.txt | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt b/Documentation/devicetree/bindings/mfd/max77686.txt
> > > index 75fdfaf41831..e39f0bc1f55e 100644
> > > --- a/Documentation/devicetree/bindings/mfd/max77686.txt
> > > +++ b/Documentation/devicetree/bindings/mfd/max77686.txt
> > > @@ -39,6 +39,12 @@ to get matched with their hardware counterparts as follow:
> > >  	-BUCKn	:	1-4.
> > >    Use standard regulator bindings for it ('regulator-off-in-suspend').
> > >  
> > > +  LDO20, LDO21, LDO22, BUCK8 and BUCK9 can be configured to GPIO enable
> > > +  control. To turn this feature on this property must be added to the regulator
> > > +  sub-node:
> > > +	- maxim,ena-gpios :	one GPIO specifier enable control (the gpio
> > > +				flags are actually ignored and always
> > > +				ACTIVE_HIGH is used)
> > 
> > How does this differ to the 'enable-gpio' property which has already
> > been defined?
> 
> Strictly speaking - no difference, just a GPIO specifier.
> 
> However the 'enable-gpio' property exists only for gpio-regulator. The
> regulator core does not support generic enable-gpio property and each
> driver implements it on its own.
> 
> Anyway Mark Brown already applied this patch to his regulator tree.

Well if he has no issue with it, then I guess I don't.

> > >  Example:
> > >  
> > > @@ -65,4 +71,12 @@ Example:
> > >  				regulator-always-on;
> > >  				regulator-boot-on;
> > >  			};
> > > +
> > > +			buck9_reg {
> > > +				regulator-compatible = "BUCK9";
> > > +				regulator-name = "CAM_ISP_CORE_1.2V";
> > > +				regulator-min-microvolt = <1000000>;
> > > +				regulator-max-microvolt = <1200000>;
> > > +				maxim,ena-gpios = <&gpm0 3 GPIO_ACTIVE_HIGH>;
> > > +			};
> > >  	}
> > 
> 

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

[PATCH v5 5/5] ARM: dts: exynos4412-trats: Switch max77686 regulators to GPIO control

[Krzysztof Kozlowski]

Remove fixed regulators (duplicating what max77686 provides) and
add GPIO enable control to max77686 regulators.

This gives the system full control over those regulators. Previously
the state of such regulators was a mixture of what max77686 driver set
over I2C and what regulator-fixed set through GPIO.

Removal of 'regulator-always-on' from CAM_ISP_CORE_1.2V (buck9) allows
disabling it when it is not used. Previously this regulator was always
enabled because its enable state is a OR of:
 - ENB9 GPIO (turned always on by regulator-fixed),
 - BUCK9EN field in BUCK9CTRL register (off by max77686 through I2C).

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 arch/arm/boot/dts/exynos4412-trats2.dts | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
index 405d4f337e89..186c210680c1 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -58,15 +58,6 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		vemmc_reg: regulator-0 {
-			compatible = "regulator-fixed";
-			regulator-name = "VMEM_VDD_2.8V";
-			regulator-min-microvolt = <2800000>;
-			regulator-max-microvolt = <2800000>;
-			gpio = <&gpk0 2 0>;
-			enable-active-high;
-		};
-
 		cam_io_reg: voltage-regulator-1 {
 			compatible = "regulator-fixed";
 			regulator-name = "CAM_SENSOR_A";
@@ -94,16 +85,6 @@
 			enable-active-high;
 		};
 
-		cam_isp_core_reg: voltage-regulator-4 {
-			compatible = "regulator-fixed";
-			regulator-name = "CAM_ISP_CORE_1.2V_EN";
-			regulator-min-microvolt = <1200000>;
-			regulator-max-microvolt = <1200000>;
-			gpio = <&gpm0 3 0>;
-			enable-active-high;
-			regulator-always-on;
-		};
-
 		ps_als_reg: voltage-regulator-5 {
 			compatible = "regulator-fixed";
 			regulator-name = "LED_A_3.0V";
@@ -405,6 +386,7 @@
 					regulator-name = "VTF_2.8V";
 					regulator-min-microvolt = <2800000>;
 					regulator-max-microvolt = <2800000>;
+					maxim,ena-gpios = <&gpy2 0 GPIO_ACTIVE_HIGH>;
 				};
 
 				ldo22_reg: ldo22 {
@@ -412,6 +394,7 @@
 					regulator-name = "VMEM_VDD_2.8V";
 					regulator-min-microvolt = <2800000>;
 					regulator-max-microvolt = <2800000>;
+					maxim,ena-gpios = <&gpk0 2 GPIO_ACTIVE_HIGH>;
 				};
 
 				ldo23_reg: ldo23 {
@@ -518,6 +501,7 @@
 					regulator-name = "VMEM_VDDF_3.0V";
 					regulator-min-microvolt = <2850000>;
 					regulator-max-microvolt = <2850000>;
+					maxim,ena-gpios = <&gpk0 2 GPIO_ACTIVE_HIGH>;
 				};
 
 				buck9_reg: buck9 {
@@ -525,6 +509,7 @@
 					regulator-name = "CAM_ISP_CORE_1.2V";
 					regulator-min-microvolt = <1000000>;
 					regulator-max-microvolt = <1200000>;
+					maxim,ena-gpios = <&gpm0 3 GPIO_ACTIVE_HIGH>;
 				};
 			};
 		};
@@ -587,7 +572,7 @@
 		broken-cd;
 		non-removable;
 		card-detect-delay = <200>;
-		vmmc-supply = <&vemmc_reg>;
+		vmmc-supply = <&ldo22_reg>;
 		clock-frequency = <400000000>;
 		samsung,dw-mshc-ciu-div = <0>;
 		samsung,dw-mshc-sdr-timing = <2 3>;
-- 
1.9.1

[PATCH v5 5/5] ARM: dts: exynos4412-trats: Switch max77686 regulators to GPIO control

[Krzysztof Kozlowski]

Hi Kukjin,

Mark Brown pulled regulator changes and documentation for bindings [1].
Could you pick up this patch?

[1] http://www.spinics.net/lists/linux-samsung-soc/msg40834.html

Best regards,
Krzysztof


On pon, 2015-01-05 at 12:48 +0100, Krzysztof Kozlowski wrote:
> Remove fixed regulators (duplicating what max77686 provides) and
> add GPIO enable control to max77686 regulators.
> 
> This gives the system full control over those regulators. Previously
> the state of such regulators was a mixture of what max77686 driver set
> over I2C and what regulator-fixed set through GPIO.
> 
> Removal of 'regulator-always-on' from CAM_ISP_CORE_1.2V (buck9) allows
> disabling it when it is not used. Previously this regulator was always
> enabled because its enable state is a OR of:
>  - ENB9 GPIO (turned always on by regulator-fixed),
>  - BUCK9EN field in BUCK9CTRL register (off by max77686 through I2C).
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>  arch/arm/boot/dts/exynos4412-trats2.dts | 25 +++++--------------------
>  1 file changed, 5 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
> index 405d4f337e89..186c210680c1 100644
> --- a/arch/arm/boot/dts/exynos4412-trats2.dts
> +++ b/arch/arm/boot/dts/exynos4412-trats2.dts
> @@ -58,15 +58,6 @@
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  
> -		vemmc_reg: regulator-0 {
> -			compatible = "regulator-fixed";
> -			regulator-name = "VMEM_VDD_2.8V";
> -			regulator-min-microvolt = <2800000>;
> -			regulator-max-microvolt = <2800000>;
> -			gpio = <&gpk0 2 0>;
> -			enable-active-high;
> -		};
> -
>  		cam_io_reg: voltage-regulator-1 {
>  			compatible = "regulator-fixed";
>  			regulator-name = "CAM_SENSOR_A";
> @@ -94,16 +85,6 @@
>  			enable-active-high;
>  		};
>  
> -		cam_isp_core_reg: voltage-regulator-4 {
> -			compatible = "regulator-fixed";
> -			regulator-name = "CAM_ISP_CORE_1.2V_EN";
> -			regulator-min-microvolt = <1200000>;
> -			regulator-max-microvolt = <1200000>;
> -			gpio = <&gpm0 3 0>;
> -			enable-active-high;
> -			regulator-always-on;
> -		};
> -
>  		ps_als_reg: voltage-regulator-5 {
>  			compatible = "regulator-fixed";
>  			regulator-name = "LED_A_3.0V";
> @@ -405,6 +386,7 @@
>  					regulator-name = "VTF_2.8V";
>  					regulator-min-microvolt = <2800000>;
>  					regulator-max-microvolt = <2800000>;
> +					maxim,ena-gpios = <&gpy2 0 GPIO_ACTIVE_HIGH>;
>  				};
>  
>  				ldo22_reg: ldo22 {
> @@ -412,6 +394,7 @@
>  					regulator-name = "VMEM_VDD_2.8V";
>  					regulator-min-microvolt = <2800000>;
>  					regulator-max-microvolt = <2800000>;
> +					maxim,ena-gpios = <&gpk0 2 GPIO_ACTIVE_HIGH>;
>  				};
>  
>  				ldo23_reg: ldo23 {
> @@ -518,6 +501,7 @@
>  					regulator-name = "VMEM_VDDF_3.0V";
>  					regulator-min-microvolt = <2850000>;
>  					regulator-max-microvolt = <2850000>;
> +					maxim,ena-gpios = <&gpk0 2 GPIO_ACTIVE_HIGH>;
>  				};
>  
>  				buck9_reg: buck9 {
> @@ -525,6 +509,7 @@
>  					regulator-name = "CAM_ISP_CORE_1.2V";
>  					regulator-min-microvolt = <1000000>;
>  					regulator-max-microvolt = <1200000>;
> +					maxim,ena-gpios = <&gpm0 3 GPIO_ACTIVE_HIGH>;
>  				};
>  			};
>  		};
> @@ -587,7 +572,7 @@
>  		broken-cd;
>  		non-removable;
>  		card-detect-delay = <200>;
> -		vmmc-supply = <&vemmc_reg>;
> +		vmmc-supply = <&ldo22_reg>;
>  		clock-frequency = <400000000>;
>  		samsung,dw-mshc-ciu-div = <0>;
>  		samsung,dw-mshc-sdr-timing = <2 3>;

[PATCH v5 0/5] regulator: Allow parsing custom DT properties with simplified DT parse

[Mark Brown]

On Mon, Jan 05, 2015 at 12:48:40PM +0100, Krzysztof Kozlowski wrote:
> Hi,
> 
> 
> The patchset adds:
> 1. a way of parsing custom DT properties by the driver when simplified
>    DT parsing method is used,
> 2. GPIO enable control to the max77686 driver.

Applied 1-4, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150108/ed295216/attachment.sig>