Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1428547 > unrolled thread

[PATCH 0/6] regulator: add enable GPIO property to pwm-regulator

Started byAlexandre Courbot <acourbot@nvidia.com>
First post2016-06-22 10:30 +0200
Last post2016-06-22 12:40 +0200
Articles 6 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/6] regulator: add enable GPIO property to pwm-regulator Alexandre Courbot <acourbot@nvidia.com> - 2016-06-22 10:30 +0200
    [PATCH 5/6] pwm-regulator: Support for enable GPIO Alexandre Courbot <acourbot@nvidia.com> - 2016-06-22 10:40 +0200
    [PATCH 4/6] regulator: gpio: Use of_get_regulator_gpio_config Alexandre Courbot <acourbot@nvidia.com> - 2016-06-22 10:40 +0200
    [PATCH 3/6] regulator: fixed: Use of_get_regulator_gpio_config Alexandre Courbot <acourbot@nvidia.com> - 2016-06-22 10:40 +0200
    [PATCH 2/6] regulator: of: Add enable GPIO configuration function Alexandre Courbot <acourbot@nvidia.com> - 2016-06-22 10:40 +0200
      Re: [PATCH 2/6] regulator: of: Add enable GPIO configuration function Mark Brown <broonie@kernel.org> - 2016-06-22 12:40 +0200

#1428547 — [PATCH 0/6] regulator: add enable GPIO property to pwm-regulator

FromAlexandre Courbot <acourbot@nvidia.com>
Date2016-06-22 10:30 +0200
Subject[PATCH 0/6] regulator: add enable GPIO property to pwm-regulator
Message-ID<rMLHQ-ZT-3@gated-at.bofh.it>
This series adds the ability for the pwm-regulator driver to have an enable-gpio
property, a feature that is required for the VDD_GPU regulator of Jetson TX1.

Before doing that though, it goes through a few required changes/improvements
in the regulator framework.

Patch 1 allows the regulator core to use both an enable GPIO and an enable
callback for a given driver. This is required for our use-case since the PWM
also needs to be enabled/disabled in pwm-regulator regardless of whether we are
using an enable GPIO or not.

Patches 2-4 factorize the enable GPIO DT parsing code. With fixed and GPIO
regulators already using that feature, and PWM to do the same, it probably
makes sense to only have one function doing this. The parsing function supports
more properties than individual drivers require, but the DT binding is the
authority on which properties are valid, not the implementation.

Finally patches 5-6 add support for the enable GPIO in pwm-regulator and the
corresponding DT binding. The binding is kept minimal (active-high and
open-drain properties can be specified in the GPIO phandle) on purpose.

Alexandre Courbot (6):
  regulator: core: Allow simultaneous use of enable op and GPIO
  regulator: of: Add enable GPIO configuration function
  regulator: fixed: Use of_get_regulator_gpio_config
  regulator: gpio: Use of_get_regulator_gpio_config
  pwm-regulator: Support for enable GPIO
  dt-bindings: pwm-regulator: Document enable-gpio property

 .../bindings/regulator/pwm-regulator.txt           |  7 ++-
 drivers/regulator/core.c                           | 34 ++++++++------
 drivers/regulator/fixed.c                          | 52 +++++++++++-----------
 drivers/regulator/gpio-regulator.c                 | 47 ++++++++++---------
 drivers/regulator/of_regulator.c                   | 52 ++++++++++++++++++++++
 drivers/regulator/pwm-regulator.c                  |  5 +++
 include/linux/regulator/of_regulator.h             | 14 ++++++
 7 files changed, 149 insertions(+), 62 deletions(-)

-- 
2.8.3

[toc] | [next] | [standalone]


#1428553 — [PATCH 5/6] pwm-regulator: Support for enable GPIO

FromAlexandre Courbot <acourbot@nvidia.com>
Date2016-06-22 10:40 +0200
Subject[PATCH 5/6] pwm-regulator: Support for enable GPIO
Message-ID<rMLRv-137-9@gated-at.bofh.it>
In reply to#1428547
Enable the pwm-regulator driver to make use of an enable GPIO.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/regulator/pwm-regulator.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index ab3cc0235843..8e5e4e744c51 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -273,6 +273,11 @@ static int pwm_regulator_probe(struct platform_device *pdev)
 	if (!init_data)
 		return -ENOMEM;
 
+	ret = of_get_regulator_gpio_config(&pdev->dev, np, "enable-gpio",
+					   &config);
+	if (ret)
+		return ret;
+
 	config.of_node = np;
 	config.dev = &pdev->dev;
 	config.driver_data = drvdata;
-- 
2.8.3

[toc] | [prev] | [next] | [standalone]


#1428555 — [PATCH 4/6] regulator: gpio: Use of_get_regulator_gpio_config

FromAlexandre Courbot <acourbot@nvidia.com>
Date2016-06-22 10:40 +0200
Subject[PATCH 4/6] regulator: gpio: Use of_get_regulator_gpio_config
Message-ID<rMLRw-137-15@gated-at.bofh.it>
In reply to#1428547
If instanciated from the DT, use of_get_regulator_gpio_config to obtain
the enable GPIO configuration.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/regulator/gpio-regulator.c | 47 ++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/drivers/regulator/gpio-regulator.c b/drivers/regulator/gpio-regulator.c
index 83e89e5d4752..5c079f854e77 100644
--- a/drivers/regulator/gpio-regulator.c
+++ b/drivers/regulator/gpio-regulator.c
@@ -153,18 +153,8 @@ of_get_gpio_regulator_config(struct device *dev, struct device_node *np,
 
 	config->supply_name = config->init_data->constraints.name;
 
-	if (of_property_read_bool(np, "enable-active-high"))
-		config->enable_high = true;
-
-	if (of_property_read_bool(np, "enable-at-boot"))
-		config->enabled_at_boot = true;
-
 	of_property_read_u32(np, "startup-delay-us", &config->startup_delay);
 
-	config->enable_gpio = of_get_named_gpio(np, "enable-gpio", 0);
-	if (config->enable_gpio == -EPROBE_DEFER)
-		return ERR_PTR(-EPROBE_DEFER);
-
 	/* Fetch GPIOs. - optional property*/
 	ret = of_gpio_count(np);
 	if ((ret < 0) && (ret != -ENOENT))
@@ -237,6 +227,9 @@ of_get_gpio_regulator_config(struct device *dev, struct device_node *np,
 				 regtype);
 	}
 
+	/* GPIO info will be obtained via regulator_of_get_gpio_config */
+	config->enable_gpio = -ENOENT;
+
 	return config;
 }
 
@@ -263,6 +256,11 @@ static int gpio_regulator_probe(struct platform_device *pdev)
 						      &drvdata->desc);
 		if (IS_ERR(config))
 			return PTR_ERR(config);
+
+		ret = of_get_regulator_gpio_config(&pdev->dev, np,
+						   "enable-gpio", &cfg);
+		if (ret)
+			return ret;
 	}
 
 	drvdata->desc.name = kstrdup(config->supply_name, GFP_KERNEL);
@@ -337,21 +335,26 @@ static int gpio_regulator_probe(struct platform_device *pdev)
 	cfg.driver_data = drvdata;
 	cfg.of_node = np;
 
+	/*
+	 * For platform-defined regulators - DT is already handled by
+	 * of_get_regulator_gpio_config
+	 */
 	if (gpio_is_valid(config->enable_gpio)) {
 		cfg.ena_gpio = config->enable_gpio;
 		cfg.ena_gpio_initialized = true;
-	}
-	cfg.ena_gpio_invert = !config->enable_high;
-	if (config->enabled_at_boot) {
-		if (config->enable_high)
-			cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
-		else
-			cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
-	} else {
-		if (config->enable_high)
-			cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
-		else
-			cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
+
+		cfg.ena_gpio_invert = !config->enable_high;
+		if (config->enabled_at_boot) {
+			if (config->enable_high)
+				cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
+			else
+				cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
+		} else {
+			if (config->enable_high)
+				cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
+			else
+				cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
+		}
 	}
 
 	drvdata->dev = regulator_register(&drvdata->desc, &cfg);
-- 
2.8.3

[toc] | [prev] | [next] | [standalone]


#1428561 — [PATCH 3/6] regulator: fixed: Use of_get_regulator_gpio_config

FromAlexandre Courbot <acourbot@nvidia.com>
Date2016-06-22 10:40 +0200
Subject[PATCH 3/6] regulator: fixed: Use of_get_regulator_gpio_config
Message-ID<rMLRw-137-27@gated-at.bofh.it>
In reply to#1428547
If instanciated from the DT, use of_get_regulator_gpio_config to obtain
the enable GPIO configuration.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/regulator/fixed.c | 52 ++++++++++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 988a7472c2ab..76a8dea763cf 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -75,22 +75,14 @@ of_get_fixed_voltage_config(struct device *dev,
 		return ERR_PTR(-EINVAL);
 	}
 
-	if (init_data->constraints.boot_on)
-		config->enabled_at_boot = true;
-
-	config->gpio = of_get_named_gpio(np, "gpio", 0);
-	if ((config->gpio < 0) && (config->gpio != -ENOENT))
-		return ERR_PTR(config->gpio);
-
 	of_property_read_u32(np, "startup-delay-us", &config->startup_delay);
 
-	config->enable_high = of_property_read_bool(np, "enable-active-high");
-	config->gpio_is_open_drain = of_property_read_bool(np,
-							   "gpio-open-drain");
-
 	if (of_find_property(np, "vin-supply", NULL))
 		config->input_supply = "vin";
 
+	/* GPIO info will be obtained via regulator_of_get_gpio_config */
+	config->gpio = -ENOENT;
+
 	return config;
 }
 
@@ -114,6 +106,12 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
 						     &drvdata->desc);
 		if (IS_ERR(config))
 			return PTR_ERR(config);
+
+		ret = of_get_regulator_gpio_config(&pdev->dev,
+						   pdev->dev.of_node, "gpio",
+						   &cfg);
+		if (ret)
+			return ret;
 	} else {
 		config = dev_get_platdata(&pdev->dev);
 	}
@@ -150,25 +148,29 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
 
 	drvdata->desc.fixed_uV = config->microvolts;
 
+	/*
+	 * For platform-defined regulators - DT is already handled by
+	 * of_get_regulator_gpio_config
+	 */
 	if (gpio_is_valid(config->gpio)) {
 		cfg.ena_gpio = config->gpio;
 		if (pdev->dev.of_node)
 			cfg.ena_gpio_initialized = true;
+		cfg.ena_gpio_invert = !config->enable_high;
+		if (config->enabled_at_boot) {
+			if (config->enable_high)
+				cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
+			else
+				cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
+		} else {
+			if (config->enable_high)
+				cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
+			else
+				cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
+		}
+		if (config->gpio_is_open_drain)
+			cfg.ena_gpio_flags |= GPIOF_OPEN_DRAIN;
 	}
-	cfg.ena_gpio_invert = !config->enable_high;
-	if (config->enabled_at_boot) {
-		if (config->enable_high)
-			cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
-		else
-			cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
-	} else {
-		if (config->enable_high)
-			cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
-		else
-			cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
-	}
-	if (config->gpio_is_open_drain)
-		cfg.ena_gpio_flags |= GPIOF_OPEN_DRAIN;
 
 	cfg.dev = &pdev->dev;
 	cfg.init_data = config->init_data;
-- 
2.8.3

[toc] | [prev] | [next] | [standalone]


#1428566 — [PATCH 2/6] regulator: of: Add enable GPIO configuration function

FromAlexandre Courbot <acourbot@nvidia.com>
Date2016-06-22 10:40 +0200
Subject[PATCH 2/6] regulator: of: Add enable GPIO configuration function
Message-ID<rMLRw-137-43@gated-at.bofh.it>
In reply to#1428547
Several regulator drivers are using an enable GPIO with similar DT
properties, yet each driver is parsing these properties its own way. Add
the of_get_regulator_gpio_config() function which is able to parse all
known properties and update the regulator_config accordingly.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/regulator/of_regulator.c       | 52 ++++++++++++++++++++++++++++++++++
 include/linux/regulator/of_regulator.h | 14 +++++++++
 2 files changed, 66 insertions(+)

diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index cd828dbf9d52..f1cff511320b 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
 #include <linux/regulator/machine.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/of_regulator.h>
@@ -350,3 +351,54 @@ struct regulator_init_data *regulator_of_get_init_data(struct device *dev,
 
 	return init_data;
 }
+
+int of_get_regulator_gpio_config(struct device *dev, struct device_node *np,
+				 const char *prop,
+				 struct regulator_config *cfg)
+{
+	enum of_gpio_flags flags;
+	bool active_high;
+	int ena_gpio;
+
+	/* Do we have an enable GPIO property? */
+	ena_gpio = of_get_named_gpio_flags(np, prop, 0, &flags);
+	if (!gpio_is_valid(ena_gpio)) {
+		/* No enable GPIO defined, nothing to do */
+		if (ena_gpio == -ENOENT)
+			return 0;
+
+		if (ena_gpio != -EPROBE_DEFER)
+			dev_err(dev, "error getting enable GPIO: %d\n",
+				ena_gpio);
+		return ena_gpio;
+	}
+
+	cfg->ena_gpio_initialized = true;
+	cfg->ena_gpio = ena_gpio;
+
+	/* Is GPIO active-low? */
+	active_high = of_property_read_bool(np, "enable-active-high");
+	cfg->ena_gpio_invert = of_property_read_bool(np, "enable-active-high") ?
+				false : !!(flags & OF_GPIO_ACTIVE_LOW);
+
+	/* Should GPIO be set? */
+	if (of_property_read_bool(np, "regulator-boot-on") ||
+	    of_property_read_bool(np, "regulator-always-on") ||
+	    of_property_read_bool(np, "enable-at-boot")) {
+		if (cfg->ena_gpio_invert)
+			cfg->ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
+		else
+			cfg->ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
+	} else {
+		if (cfg->ena_gpio_invert)
+			cfg->ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
+		else
+			cfg->ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
+	}
+
+	if (of_property_read_bool(np, "gpio_open_drain"))
+		cfg->ena_gpio_flags |= GPIOF_OPEN_DRAIN;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_regulator_gpio_config);
diff --git a/include/linux/regulator/of_regulator.h b/include/linux/regulator/of_regulator.h
index 763953f7e3b8..eb4f1b6a26ba 100644
--- a/include/linux/regulator/of_regulator.h
+++ b/include/linux/regulator/of_regulator.h
@@ -7,6 +7,7 @@
 #define __LINUX_OF_REG_H
 
 struct regulator_desc;
+struct regulator_config;
 
 struct of_regulator_match {
 	const char *name;
@@ -24,6 +25,10 @@ extern struct regulator_init_data
 extern int of_regulator_match(struct device *dev, struct device_node *node,
 			      struct of_regulator_match *matches,
 			      unsigned int num_matches);
+extern int of_get_regulator_gpio_config(struct device *dev,
+					struct device_node *node,
+					const char *prop,
+					struct regulator_config *config);
 #else
 static inline struct regulator_init_data
 	*of_get_regulator_init_data(struct device *dev,
@@ -40,6 +45,15 @@ static inline int of_regulator_match(struct device *dev,
 {
 	return 0;
 }
+
+static inline int of_get_regulator_gpio_config(struct device *dev,
+					       struct device_node *node,
+					       const char *prop,
+					       struct regulator_config *config)
+{
+	return 0;
+}
+
 #endif /* CONFIG_OF */
 
 #endif /* __LINUX_OF_REG_H */
-- 
2.8.3

[toc] | [prev] | [next] | [standalone]


#1428676 — Re: [PATCH 2/6] regulator: of: Add enable GPIO configuration function

FromMark Brown <broonie@kernel.org>
Date2016-06-22 12:40 +0200
SubjectRe: [PATCH 2/6] regulator: of: Add enable GPIO configuration function
Message-ID<rMNJE-2bS-41@gated-at.bofh.it>
In reply to#1428566

[Multipart message — attachments visible in raw view] — view raw

On Wed, Jun 22, 2016 at 05:25:54PM +0900, Alexandre Courbot wrote:
> Several regulator drivers are using an enable GPIO with similar DT
> properties, yet each driver is parsing these properties its own way. Add
> the of_get_regulator_gpio_config() function which is able to parse all
> known properties and update the regulator_config accordingly.

Why is this a function and not something data driven in the core?

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web