[PATCH 1/2] regulator: core: Correct type of mode in regulator_mode_constrain

[PATCH 1/2] regulator: core: Correct type of mode in regulator_mode_constrain

[Charles Keepax]

Every function handling the mode within the regulator core uses an unsigned
int for mode, except for regulator_mode_constrain. This patch changes the
type of mode within regulator_mode_constrain which fixes several instances
where we are passing pointers to unsigned ints then treating them as an int
within this function.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/regulator/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index cd864a7..b3417ac 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -293,7 +293,8 @@ static int regulator_check_current_limit(struct regulator_dev *rdev,
 }
 
 /* operating mode constraint check */
-static int regulator_mode_constrain(struct regulator_dev *rdev, int *mode)
+static int regulator_mode_constrain(struct regulator_dev *rdev,
+				    unsigned int *mode)
 {
 	switch (*mode) {
 	case REGULATOR_MODE_FAST:
-- 
2.1.4

[PATCH 2/2] regulator: core: Allow enable GPIO to be specified using GPIOD

[Charles Keepax]

The regulator subsystem has used GPIOs internally for a while, however,
end drivers must still specify their enable GPIO using a GPIO number. This
patch allows the end drivers to specify the enable GPIO using GPIOD
directly.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/regulator/core.c         | 38 ++++++++++++++++++++++++++------------
 include/linux/regulator/driver.h | 14 +++++++++-----
 2 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b3417ac..48f2813 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1929,6 +1929,16 @@ void regulator_bulk_unregister_supply_alias(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(regulator_bulk_unregister_supply_alias);
 
+static bool regulator_ena_gpio_valid(const struct regulator_config *config)
+{
+	if (config->ena_gpiod)
+		return true;
+
+	if (config->ena_gpio || config->ena_gpio_initialized)
+		return gpio_is_valid(config->ena_gpio);
+
+	return false;
+}
 
 /* Manage enable GPIO list. Same GPIO pin can be shared among regulators */
 static int regulator_ena_gpio_request(struct regulator_dev *rdev,
@@ -1938,7 +1948,10 @@ static int regulator_ena_gpio_request(struct regulator_dev *rdev,
 	struct gpio_desc *gpiod;
 	int ret;
 
-	gpiod = gpio_to_desc(config->ena_gpio);
+	if (config->ena_gpiod)
+		gpiod = config->ena_gpiod;
+	else
+		gpiod = gpio_to_desc(config->ena_gpio);
 
 	list_for_each_entry(pin, &regulator_ena_gpio_list, list) {
 		if (pin->gpiod == gpiod) {
@@ -1948,16 +1961,18 @@ static int regulator_ena_gpio_request(struct regulator_dev *rdev,
 		}
 	}
 
-	ret = gpio_request_one(config->ena_gpio,
-				GPIOF_DIR_OUT | config->ena_gpio_flags,
-				rdev_get_name(rdev));
-	if (ret)
-		return ret;
-
 	pin = kzalloc(sizeof(struct regulator_enable_gpio), GFP_KERNEL);
-	if (pin == NULL) {
-		gpio_free(config->ena_gpio);
+	if (!pin)
 		return -ENOMEM;
+
+	if (!config->ena_gpiod) {
+		ret = gpio_request_one(config->ena_gpio,
+				       GPIOF_DIR_OUT | config->ena_gpio_flags,
+				       rdev_get_name(rdev));
+		if (ret) {
+			kfree(pin);
+			return ret;
+		}
 	}
 
 	pin->gpiod = gpiod;
@@ -4027,14 +4042,13 @@ regulator_register(const struct regulator_desc *regulator_desc,
 			goto clean;
 	}
 
-	if ((config->ena_gpio || config->ena_gpio_initialized) &&
-	    gpio_is_valid(config->ena_gpio)) {
+	if (regulator_ena_gpio_valid(config)) {
 		mutex_lock(&regulator_list_mutex);
 		ret = regulator_ena_gpio_request(rdev, config);
 		mutex_unlock(&regulator_list_mutex);
 		if (ret != 0) {
 			rdev_err(rdev, "Failed to request enable GPIO%d: %d\n",
-				 config->ena_gpio, ret);
+				 desc_to_gpio(config->ena_gpiod), ret);
 			goto clean;
 		}
 	}
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index dac8e7b1..0a3f3a0 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -367,11 +367,13 @@ struct regulator_desc {
  *           NULL).
  * @regmap: regmap to use for core regmap helpers if dev_get_regmap() is
  *          insufficient.
- * @ena_gpio_initialized: GPIO controlling regulator enable was properly
- *                        initialized, meaning that >= 0 is a valid gpio
- *                        identifier and < 0 is a non existent gpio.
- * @ena_gpio: GPIO controlling regulator enable.
+ * @ena_gpiod: GPIO controlling regulator enable.
  * @ena_gpio_invert: Sense for GPIO enable control.
+ * @ena_gpio_initialized: ena_gpio enable was properly initialized,
+ *                        meaning that >= 0 is a valid gpio identifier
+ *                        and < 0 is a non existent gpio.
+ * @ena_gpio: GPIO controlling regulator enable, deprecated in favour of
+ *            ena_gpiod.
  * @ena_gpio_flags: Flags to use when calling gpio_request_one()
  */
 struct regulator_config {
@@ -381,9 +383,11 @@ struct regulator_config {
 	struct device_node *of_node;
 	struct regmap *regmap;
 
+	struct gpio_desc *ena_gpiod;
+	unsigned int ena_gpio_invert:1;
+
 	bool ena_gpio_initialized;
 	int ena_gpio;
-	unsigned int ena_gpio_invert:1;
 	unsigned int ena_gpio_flags;
 };
 
-- 
2.1.4

Re: [PATCH 2/2] regulator: core: Allow enable GPIO to be specified using GPIOD

[Charles Keepax]

On Tue, Nov 29, 2016 at 11:30:45AM +0000, Charles Keepax wrote:
> The regulator subsystem has used GPIOs internally for a while, however,
> end drivers must still specify their enable GPIO using a GPIO number. This
> patch allows the end drivers to specify the enable GPIO using GPIOD
> directly.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/regulator/core.c         | 38 ++++++++++++++++++++++++++------------
>  include/linux/regulator/driver.h | 14 +++++++++-----
>  2 files changed, 35 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index b3417ac..48f2813 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
>  
>  	pin->gpiod = gpiod;
> @@ -4027,14 +4042,13 @@ regulator_register(const struct regulator_desc *regulator_desc,
>  			goto clean;
>  	}
>  
> -	if ((config->ena_gpio || config->ena_gpio_initialized) &&
> -	    gpio_is_valid(config->ena_gpio)) {
> +	if (regulator_ena_gpio_valid(config)) {
>  		mutex_lock(&regulator_list_mutex);
>  		ret = regulator_ena_gpio_request(rdev, config);
>  		mutex_unlock(&regulator_list_mutex);
>  		if (ret != 0) {
>  			rdev_err(rdev, "Failed to request enable GPIO%d: %d\n",
> -				 config->ena_gpio, ret);
> +				 desc_to_gpio(config->ena_gpiod), ret);

Ah sorry small bug here I will respin.

Thanks,
Charles

Re: [PATCH 2/2] regulator: core: Allow enable GPIO to be specified using GPIOD

[Mark Brown]

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

On Tue, Nov 29, 2016 at 11:30:45AM +0000, Charles Keepax wrote:
> The regulator subsystem has used GPIOs internally for a while, however,
> end drivers must still specify their enable GPIO using a GPIO number. This
> patch allows the end drivers to specify the enable GPIO using GPIOD
> directly.

The reason we don't support descriptors is that the GPIO API currently
doesn't support having more than one user of a descriptor but GPIO
enables for regulators get shared often enough to be interesting.  If
drivers start using this then that'd mean that the shared enable support
would become erratic depending on which driver was in use.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [PATCH 2/2] regulator: core: Allow enable GPIO to be specified using GPIOD

[Charles Keepax]

On Tue, Nov 29, 2016 at 11:45:47AM +0000, Mark Brown wrote:
> On Tue, Nov 29, 2016 at 11:30:45AM +0000, Charles Keepax wrote:
> > The regulator subsystem has used GPIOs internally for a while, however,
> > end drivers must still specify their enable GPIO using a GPIO number. This
> > patch allows the end drivers to specify the enable GPIO using GPIOD
> > directly.
> 
> The reason we don't support descriptors is that the GPIO API currently
> doesn't support having more than one user of a descriptor but GPIO
> enables for regulators get shared often enough to be interesting.  If
> drivers start using this then that'd mean that the shared enable support
> would become erratic depending on which driver was in use.

Ah ok thanks, guess we can drop this one then. The first patch in
the chain is hopefully still good though.

Thanks,
Charles

Applied "regulator: core: Correct type of mode in regulator_mode_constrain" to the regulator tree

[Mark Brown]

The patch

   regulator: core: Correct type of mode in regulator_mode_constrain

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 109c75afa1cf7c08015f19e354bed581f29f7a94 Mon Sep 17 00:00:00 2001
From: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Date: Tue, 29 Nov 2016 11:50:03 +0000
Subject: [PATCH] regulator: core: Correct type of mode in
 regulator_mode_constrain

Every function handling the mode within the regulator core uses an unsigned
int for mode, except for regulator_mode_constrain. This patch changes the
type of mode within regulator_mode_constrain which fixes several instances
where we are passing pointers to unsigned ints then treating them as an int
within this function.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 67426c0477d3..b6b3aa8ef5db 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -293,7 +293,8 @@ static int regulator_check_current_limit(struct regulator_dev *rdev,
 }
 
 /* operating mode constraint check */
-static int regulator_mode_constrain(struct regulator_dev *rdev, int *mode)
+static int regulator_mode_constrain(struct regulator_dev *rdev,
+				    unsigned int *mode)
 {
 	switch (*mode) {
 	case REGULATOR_MODE_FAST:
-- 
2.10.2