regulator: core: Correct type of mode in regulator_mode_constrain

PATCH Superseded Charles Keepax 2 participants 6 messages Nov 29, 2016 – Nov 30, 2016 Voltage and current regulator framework

lore

Share & tools

lore

Charles Keepax

author

2016-11-29 11:30 UTC #0 lore raw

PATCH 1/2 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 <[email protected]>

---
 drivers/regulator/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/regulator/core.c +2 -1

index cd864a7..b3417ac 100644
@@ -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:

Signature · 1 line

2.1.4

Charles Keepax

author

2016-11-29 11:30 UTC #1 lore raw

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

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 <[email protected]>

---
 drivers/regulator/core.c         | 38 ++++++++++++++++++++++++++------------
 include/linux/regulator/driver.h | 14 +++++++++-----
 2 files changed, 35 insertions(+), 17 deletions(-)

drivers/regulator/core.c +26 ~~-12~~

index b3417ac..48f2813 100644
@@ -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;
 		}
 	}

include/linux/regulator/driver.h +9 -5

index dac8e7b1..0a3f3a0 100644
@@ -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;
 };

Signature · 1 line

2.1.4

Charles Keepax

author

2016-11-29 11:38 UTC #2 lore raw

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

Quoted 21 lines · 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 <[email protected]>
---
 drivers/regulator/core.c         | 38 ++++++++++++++++++++++++++------------
 include/linux/regulator/driver.h | 14 +++++++++-----
 2 files changed, 35 insertions(+), 17 deletions(-)
drivers/regulator/core.c +0 -0
index b3417ac..48f2813 100644
 
 	pin->gpiod = gpiod;
@@ -4027,14 +4042,13 @@ regulator_register(const struct regulator_desc *regulator_desc,
 			goto clean;
 	}
 

drivers/regulator/core.c regulator_register(const struct regulator_desc *regulator_desc, · line 4051

Quoted 10 lines

drivers/regulator/core.c +2 -3

-	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

Mark Brown

maintainer

2016-11-29 11:46 UTC #3 lore raw

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

In reply to Charles Keepax

Quoted 4 lines · 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.

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.

Charles Keepax

author

2016-11-29 11:52 UTC #4 lore raw

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

Quoted 11 lines · Mark Brown

Quoted 4 lines · 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.
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

Mark Brown

maintainer

2016-11-30 18:08 UTC #5 lore raw

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

In reply to Charles Keepax

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

Quoted 1 line

From 109c75afa1cf7c08015f19e354bed581f29f7a94 Mon Sep 17 00:00:00 2001

From: Charles Keepax <[email protected]>
Date: Tue, 29 Nov 2016 11:50:03 +0000
Subject: [PATCH] regulator: core: Correct type of mode in
regulator_mode_constrain

Signed-off-by Charles Keepax <[email protected]>

Signed-off-by Mark Brown <[email protected]>

---
 drivers/regulator/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/regulator/core.c +2 -1

index 67426c0477d3..b6b3aa8ef5db 100644
@@ -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:

Signature · 1 line

2.10.2