2020-11-13 00:20:46

by Michał Mirosław

Subject: [PATCH 0/4] regulator: debugging and fixing supply deps

It turns out that commit aea6cb99703e ("regulator: resolve supply
after creating regulator") exposed a number of issues in regulator
initialization and introduced a memory leak of its own. One uncovered
problem was already fixed by cf1ad559a20d ("regulator: defer probe when
trying to get voltage from unresolved supply"). This series fixes the
remaining ones and adds a two debugging aids to help in the future.

The final patch adds a workaround to preexisting problem occurring with
regulators that have the same name as its supply_name. This worked
before by accident, so might be worth backporting. The error message is
left on purpose so that these configurations can be detected and fixed.

(The first two patches are resends from Nov 5).

Michał Mirosław (4):
regulator: fix memory leak with repeated set_machine_constraints()
regulator: debug early supply resolving
regulator: avoid resolve_supply() infinite recursion
regulator: workaround self-referent regulators

drivers/regulator/core.c | 40 ++++++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 16 deletions(-)

--
2.20.1

2020-11-13 00:21:24

by Michał Mirosław

Subject: [PATCH 1/4] regulator: fix memory leak with repeated set_machine_constraints()

Fixed commit introduced a possible second call to
set_machine_constraints() and that allocates memory for
rdev->constraints. Move the allocation to the caller so
it's easier to manage and done once.

Fixes: aea6cb99703e ("regulator: resolve supply after creating regulator")
Cc: [email protected]
Signed-off-by: Michał Mirosław <[email protected]>
---
drivers/regulator/core.c | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 2e1ea18221ef..bcd64ba21fb9 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1315,7 +1315,6 @@ static int _regulator_do_enable(struct regulator_dev *rdev);
/**
* set_machine_constraints - sets regulator constraints
* @rdev: regulator source
- * @constraints: constraints to apply
*
* Allows platform initialisation code to define and constrain
* regulator circuits e.g. valid voltage/current ranges, etc. NOTE:
@@ -1323,21 +1322,11 @@ static int _regulator_do_enable(struct regulator_dev *rdev);
* regulator operations to proceed i.e. set_voltage, set_current_limit,
* set_mode.
*/
-static int set_machine_constraints(struct regulator_dev *rdev,
- const struct regulation_constraints *constraints)
+static int set_machine_constraints(struct regulator_dev *rdev)
{
int ret = 0;
const struct regulator_ops *ops = rdev->desc->ops;

- if (constraints)
- rdev->constraints = kmemdup(constraints, sizeof(*constraints),
- GFP_KERNEL);
- else
- rdev->constraints = kzalloc(sizeof(*constraints),
- GFP_KERNEL);
- if (!rdev->constraints)
- return -ENOMEM;
-
ret = machine_constraints_voltage(rdev, rdev->constraints);
if (ret != 0)
return ret;
@@ -5146,7 +5135,6 @@ struct regulator_dev *
regulator_register(const struct regulator_desc *regulator_desc,
const struct regulator_config *cfg)
{
- const struct regulation_constraints *constraints = NULL;
const struct regulator_init_data *init_data;
struct regulator_config *config = NULL;
static atomic_t regulator_no = ATOMIC_INIT(-1);
@@ -5285,14 +5273,23 @@ regulator_register(const struct regulator_desc *regulator_desc,

/* set regulator constraints */
if (init_data)
- constraints = &init_data->constraints;
+ rdev->constraints = kmemdup(&init_data->constraints,
+ sizeof(*rdev->constraints),
+ GFP_KERNEL);
+ else
+ rdev->constraints = kzalloc(sizeof(*rdev->constraints),
+ GFP_KERNEL);
+ if (!rdev->constraints) {
+ ret = -ENOMEM;
+ goto wash;
+ }

if (init_data && init_data->supply_regulator)
rdev->supply_name = init_data->supply_regulator;
else if (regulator_desc->supply_name)
rdev->supply_name = regulator_desc->supply_name;

- ret = set_machine_constraints(rdev, constraints);
+ ret = set_machine_constraints(rdev);
if (ret == -EPROBE_DEFER) {
/* Regulator might be in bypass mode and so needs its supply
* to set the constraints */
@@ -5301,7 +5298,7 @@ regulator_register(const struct regulator_desc *regulator_desc,
* that is just being created */
ret = regulator_resolve_supply(rdev);
if (!ret)
- ret = set_machine_constraints(rdev, constraints);
+ ret = set_machine_constraints(rdev);
else
rdev_dbg(rdev, "unable to resolve supply early: %pe\n",
ERR_PTR(ret));
--
2.20.1