regulator: core: Track dangling GPIO descriptors

If a GPIO descriptor is passed to the regulator_register()
function inside the config->ena_gpiod callers must be
sure that once they call this API the regulator core
owns that descriptor and will make sure to issue
gpiod_put() on it, no matter whether the call is
successful or not.

For device tree regulators, the regulator core will
automatically set up regulator init data from the device
tree when registering a regulator by calling
regulator_of_get_init_data() which in turn calls down to
the regulator driver's .of_parse_cb() callback.
This callback (in drivers such as for max77686) may also
choose to fill in the config->ena_gpiod field with a GPIO
descriptor.

Harden the errorpath of regulator_register() to
properly gpiod_put() any passed in cfg->ena_gpiod
or any gpiod coming from the device tree on any type
of error.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
This commit is contained in:
Linus Walleij 2018-12-06 13:43:37 +01:00 committed by Mark Brown
parent c32569e358
commit 0edb040d41
No known key found for this signature in database
GPG key ID: 24D68B725D5487D0

View file

@ -4868,21 +4868,33 @@ regulator_register(const struct regulator_desc *regulator_desc,
struct regulator_config *config = NULL; struct regulator_config *config = NULL;
static atomic_t regulator_no = ATOMIC_INIT(-1); static atomic_t regulator_no = ATOMIC_INIT(-1);
struct regulator_dev *rdev; struct regulator_dev *rdev;
bool dangling_cfg_gpiod = false;
bool dangling_of_gpiod = false;
struct device *dev; struct device *dev;
int ret, i; int ret, i;
if (regulator_desc == NULL || cfg == NULL) if (cfg == NULL)
return ERR_PTR(-EINVAL); return ERR_PTR(-EINVAL);
if (cfg->ena_gpiod)
dangling_cfg_gpiod = true;
if (regulator_desc == NULL) {
ret = -EINVAL;
goto rinse;
}
dev = cfg->dev; dev = cfg->dev;
WARN_ON(!dev); WARN_ON(!dev);
if (regulator_desc->name == NULL || regulator_desc->ops == NULL) if (regulator_desc->name == NULL || regulator_desc->ops == NULL) {
return ERR_PTR(-EINVAL); ret = -EINVAL;
goto rinse;
}
if (regulator_desc->type != REGULATOR_VOLTAGE && if (regulator_desc->type != REGULATOR_VOLTAGE &&
regulator_desc->type != REGULATOR_CURRENT) regulator_desc->type != REGULATOR_CURRENT) {
return ERR_PTR(-EINVAL); ret = -EINVAL;
goto rinse;
}
/* Only one of each should be implemented */ /* Only one of each should be implemented */
WARN_ON(regulator_desc->ops->get_voltage && WARN_ON(regulator_desc->ops->get_voltage &&
@ -4893,16 +4905,20 @@ regulator_register(const struct regulator_desc *regulator_desc,
/* If we're using selectors we must implement list_voltage. */ /* If we're using selectors we must implement list_voltage. */
if (regulator_desc->ops->get_voltage_sel && if (regulator_desc->ops->get_voltage_sel &&
!regulator_desc->ops->list_voltage) { !regulator_desc->ops->list_voltage) {
return ERR_PTR(-EINVAL); ret = -EINVAL;
goto rinse;
} }
if (regulator_desc->ops->set_voltage_sel && if (regulator_desc->ops->set_voltage_sel &&
!regulator_desc->ops->list_voltage) { !regulator_desc->ops->list_voltage) {
return ERR_PTR(-EINVAL); ret = -EINVAL;
goto rinse;
} }
rdev = kzalloc(sizeof(struct regulator_dev), GFP_KERNEL); rdev = kzalloc(sizeof(struct regulator_dev), GFP_KERNEL);
if (rdev == NULL) if (rdev == NULL) {
return ERR_PTR(-ENOMEM); ret = -ENOMEM;
goto rinse;
}
/* /*
* Duplicate the config so the driver could override it after * Duplicate the config so the driver could override it after
@ -4911,11 +4927,22 @@ regulator_register(const struct regulator_desc *regulator_desc,
config = kmemdup(cfg, sizeof(*cfg), GFP_KERNEL); config = kmemdup(cfg, sizeof(*cfg), GFP_KERNEL);
if (config == NULL) { if (config == NULL) {
kfree(rdev); kfree(rdev);
return ERR_PTR(-ENOMEM); ret = -ENOMEM;
goto rinse;
} }
init_data = regulator_of_get_init_data(dev, regulator_desc, config, init_data = regulator_of_get_init_data(dev, regulator_desc, config,
&rdev->dev.of_node); &rdev->dev.of_node);
/*
* We need to keep track of any GPIO descriptor coming from the
* device tree until we have handled it over to the core. If the
* config that was passed in to this function DOES NOT contain
* a descriptor, and the config after this call DOES contain
* a descriptor, we definately got one from parsing the device
* tree.
*/
if (!cfg->ena_gpiod && config->ena_gpiod)
dangling_of_gpiod = true;
if (!init_data) { if (!init_data) {
init_data = config->init_data; init_data = config->init_data;
rdev->dev.of_node = of_node_get(config->of_node); rdev->dev.of_node = of_node_get(config->of_node);
@ -4954,6 +4981,9 @@ regulator_register(const struct regulator_desc *regulator_desc,
config->ena_gpio, ret); config->ena_gpio, ret);
goto clean; goto clean;
} }
/* The regulator core took over the GPIO descriptor */
dangling_cfg_gpiod = false;
dangling_of_gpiod = false;
} }
/* register with sysfs */ /* register with sysfs */
@ -5039,8 +5069,13 @@ regulator_register(const struct regulator_desc *regulator_desc,
regulator_ena_gpio_free(rdev); regulator_ena_gpio_free(rdev);
mutex_unlock(&regulator_list_mutex); mutex_unlock(&regulator_list_mutex);
clean: clean:
if (dangling_of_gpiod)
gpiod_put(config->ena_gpiod);
kfree(rdev); kfree(rdev);
kfree(config); kfree(config);
rinse:
if (dangling_cfg_gpiod)
gpiod_put(cfg->ena_gpiod);
return ERR_PTR(ret); return ERR_PTR(ret);
} }
EXPORT_SYMBOL_GPL(regulator_register); EXPORT_SYMBOL_GPL(regulator_register);