Skip to content

Commit

Permalink
Devm helpers for regulator get and enable
Browse files Browse the repository at this point in the history
Merge series from Matti Vaittinen <mazziesaccount@gmail.com>:

A few* drivers seem to use pattern demonstrated by pseudocode:

- devm_regulator_get()
- regulator_enable()
- devm_add_action_or_reset(regulator_disable())

Introducing devm helpers for this pattern would remove bunch of code from
drivers. Typically following:

- replace 3 calls (devm_regulator_get[_optional](), regulator_enable(),
  devm_add_action_or_reset()) with just one
  (devm_regulator_get_enable[_optional]()).
- drop disable callback.
- remove stored pointer to struct regulator - which can lead to problem
  when an devm action for regulator_disable is used.

I believe this simplifies things by removing some dublicated code.

The suggested managed 'get_enable' APIs do not return the pointer to
regulators for user because any call to regulator_disable()
(or regulator_enable()) may easily lead to regulator enable count imbalance
upon device detach. (Eg, if someone calls regulator_disable() and the
device is then detached before user has re-enabled the regulator). Not
returning the pointer to obtained regulator to caller is a good hint that
the enable/disable should not be manually handled when these APIs are used.

OTOH, not returning the pointer reduces the use-cases by not allowing
the consumers to perform other regulator actions. For example request the
voltages. A few drivers which used the "get, enable,
devm_action_to_disable" did also query the voltages. The API does not suit
needs of such users.
  • Loading branch information
Mark Brown committed Aug 18, 2022
2 parents 9b6744f + da279e6 commit ee94aff
Show file tree
Hide file tree
Showing 2 changed files with 191 additions and 0 deletions.
164 changes: 164 additions & 0 deletions drivers/regulator/devres.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,65 @@ struct regulator *devm_regulator_get_exclusive(struct device *dev,
}
EXPORT_SYMBOL_GPL(devm_regulator_get_exclusive);

static void regulator_action_disable(void *d)
{
struct regulator *r = (struct regulator *)d;

regulator_disable(r);
}

static int _devm_regulator_get_enable(struct device *dev, const char *id,
int get_type)
{
struct regulator *r;
int ret;

r = _devm_regulator_get(dev, id, get_type);
if (IS_ERR(r))
return PTR_ERR(r);

ret = regulator_enable(r);
if (!ret)
ret = devm_add_action_or_reset(dev, &regulator_action_disable, r);

if (ret)
devm_regulator_put(r);

return ret;
}

/**
* devm_regulator_get_enable_optional - Resource managed regulator get and enable
* @dev: device to supply
* @id: supply name or regulator ID.
*
* Get and enable regulator for duration of the device life-time.
* regulator_disable() and regulator_put() are automatically called on driver
* detach. See regulator_get_optional() and regulator_enable() for more
* information.
*/
int devm_regulator_get_enable_optional(struct device *dev, const char *id)
{
return _devm_regulator_get_enable(dev, id, OPTIONAL_GET);
}
EXPORT_SYMBOL_GPL(devm_regulator_get_enable_optional);

/**
* devm_regulator_get_enable - Resource managed regulator get and enable
* @dev: device to supply
* @id: supply name or regulator ID.
*
* Get and enable regulator for duration of the device life-time.
* regulator_disable() and regulator_put() are automatically called on driver
* detach. See regulator_get() and regulator_enable() for more
* information.
*/
int devm_regulator_get_enable(struct device *dev, const char *id)
{
return _devm_regulator_get_enable(dev, id, NORMAL_GET);
}
EXPORT_SYMBOL_GPL(devm_regulator_get_enable);

/**
* devm_regulator_get_optional - Resource managed regulator_get_optional()
* @dev: device to supply
Expand Down Expand Up @@ -194,6 +253,111 @@ int devm_regulator_bulk_get_const(struct device *dev, int num_consumers,
}
EXPORT_SYMBOL_GPL(devm_regulator_bulk_get_const);

static int devm_regulator_bulk_match(struct device *dev, void *res,
void *data)
{
struct regulator_bulk_devres *match = res;
struct regulator_bulk_data *target = data;

/*
* We check the put uses same consumer list as the get did.
* We _could_ scan all entries in consumer array and check the
* regulators match but ATM I don't see the need. We can change this
* later if needed.
*/
return match->consumers == target;
}

/**
* devm_regulator_bulk_put - Resource managed regulator_bulk_put()
* @consumers: consumers to free
*
* Deallocate regulators allocated with devm_regulator_bulk_get(). Normally
* this function will not need to be called and the resource management
* code will ensure that the resource is freed.
*/
void devm_regulator_bulk_put(struct regulator_bulk_data *consumers)
{
int rc;
struct regulator *regulator = consumers[0].consumer;

rc = devres_release(regulator->dev, devm_regulator_bulk_release,
devm_regulator_bulk_match, consumers);
if (rc != 0)
WARN_ON(rc);
}
EXPORT_SYMBOL_GPL(devm_regulator_bulk_put);

static void devm_regulator_bulk_disable(void *res)
{
struct regulator_bulk_devres *devres = res;
int i;

for (i = 0; i < devres->num_consumers; i++)
regulator_disable(devres->consumers[i].consumer);
}

/**
* devm_regulator_bulk_get_enable - managed get'n enable multiple regulators
*
* @dev: device to supply
* @num_consumers: number of consumers to register
* @id: list of supply names or regulator IDs
*
* @return 0 on success, an errno on failure.
*
* This helper function allows drivers to get several regulator
* consumers in one operation with management, the regulators will
* automatically be freed when the device is unbound. If any of the
* regulators cannot be acquired then any regulators that were
* allocated will be freed before returning to the caller.
*/
int devm_regulator_bulk_get_enable(struct device *dev, int num_consumers,
const char * const *id)
{
struct regulator_bulk_devres *devres;
struct regulator_bulk_data *consumers;
int i, ret;

devres = devm_kmalloc(dev, sizeof(*devres), GFP_KERNEL);
if (!devres)
return -ENOMEM;

devres->consumers = devm_kcalloc(dev, num_consumers, sizeof(*consumers),
GFP_KERNEL);
consumers = devres->consumers;
if (!consumers)
return -ENOMEM;

devres->num_consumers = num_consumers;

for (i = 0; i < num_consumers; i++)
consumers[i].supply = id[i];

ret = devm_regulator_bulk_get(dev, num_consumers, consumers);
if (ret)
return ret;

for (i = 0; i < num_consumers; i++) {
ret = regulator_enable(consumers[i].consumer);
if (ret)
goto unwind;
}

ret = devm_add_action(dev, devm_regulator_bulk_disable, devres);
if (!ret)
return 0;

unwind:
while (--i >= 0)
regulator_disable(consumers[i].consumer);

devm_regulator_bulk_put(consumers);

return ret;
}
EXPORT_SYMBOL_GPL(devm_regulator_bulk_get_enable);

static void devm_rdev_release(struct device *dev, void *res)
{
regulator_unregister(*(struct regulator_dev **)res);
Expand Down
27 changes: 27 additions & 0 deletions include/linux/regulator/consumer.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ struct regulator *__must_check regulator_get_optional(struct device *dev,
const char *id);
struct regulator *__must_check devm_regulator_get_optional(struct device *dev,
const char *id);
int devm_regulator_get_enable(struct device *dev, const char *id);
int devm_regulator_get_enable_optional(struct device *dev, const char *id);
void regulator_put(struct regulator *regulator);
void devm_regulator_put(struct regulator *regulator);

Expand Down Expand Up @@ -244,12 +246,15 @@ int __must_check regulator_bulk_get(struct device *dev, int num_consumers,
struct regulator_bulk_data *consumers);
int __must_check devm_regulator_bulk_get(struct device *dev, int num_consumers,
struct regulator_bulk_data *consumers);
void devm_regulator_bulk_put(struct regulator_bulk_data *consumers);
int __must_check devm_regulator_bulk_get_const(
struct device *dev, int num_consumers,
const struct regulator_bulk_data *in_consumers,
struct regulator_bulk_data **out_consumers);
int __must_check regulator_bulk_enable(int num_consumers,
struct regulator_bulk_data *consumers);
int devm_regulator_bulk_get_enable(struct device *dev, int num_consumers,
const char * const *id);
int regulator_bulk_disable(int num_consumers,
struct regulator_bulk_data *consumers);
int regulator_bulk_force_disable(int num_consumers,
Expand Down Expand Up @@ -354,6 +359,17 @@ devm_regulator_get_exclusive(struct device *dev, const char *id)
return ERR_PTR(-ENODEV);
}

static inline int devm_regulator_get_enable(struct device *dev, const char *id)
{
return -ENODEV;
}

static inline int devm_regulator_get_enable_optional(struct device *dev,
const char *id)
{
return -ENODEV;
}

static inline struct regulator *__must_check
regulator_get_optional(struct device *dev, const char *id)
{
Expand All @@ -375,6 +391,10 @@ static inline void devm_regulator_put(struct regulator *regulator)
{
}

static inline void devm_regulator_bulk_put(struct regulator_bulk_data *consumers)
{
}

static inline int regulator_register_supply_alias(struct device *dev,
const char *id,
struct device *alias_dev,
Expand Down Expand Up @@ -465,6 +485,13 @@ static inline int regulator_bulk_enable(int num_consumers,
return 0;
}

static inline int devm_regulator_bulk_get_enable(struct device *dev,
int num_consumers,
const char * const *id)
{
return 0;
}

static inline int regulator_bulk_disable(int num_consumers,
struct regulator_bulk_data *consumers)
{
Expand Down

0 comments on commit ee94aff

Please sign in to comment.