* [PATCH 3/4] regulator: avoid resolve_supply() infinite recursion
2020-11-13 0:16 [PATCH 0/4] regulator: debugging and fixing supply deps Michał Mirosław
@ 2020-11-13 0:16 ` Michał Mirosław
2020-11-13 0:16 ` [PATCH 2/4] regulator: debug early supply resolving Michał Mirosław
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Michał Mirosław @ 2020-11-13 0:16 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown; +Cc: Ahmad Fatoum, linux-kernel, linux-arm-kernel
When a regulator's name equals its supply's name the
regulator_resolve_supply() recurses indefinitely. Add a check
so that debugging the problem is easier. The "fixed" commit
just exposed the problem.
Fixes: aea6cb99703e ("regulator: resolve supply after creating regulator")
Cc: stable@vger.kernel.org
Reported-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/regulator/core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index ad36f03d7ee6..ab922ed273f3 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1841,6 +1841,12 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
}
}
+ if (r == rdev) {
+ dev_err(dev, "Supply for %s (%s) resolved to itself\n",
+ rdev->desc->name, rdev->supply_name);
+ return -EINVAL;
+ }
+
/*
* If the supply's parent device is not the same as the
* regulator's parent device, then ensure the parent device
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/4] regulator: debug early supply resolving
2020-11-13 0:16 [PATCH 0/4] regulator: debugging and fixing supply deps Michał Mirosław
2020-11-13 0:16 ` [PATCH 3/4] regulator: avoid resolve_supply() infinite recursion Michał Mirosław
@ 2020-11-13 0:16 ` Michał Mirosław
2020-11-13 0:16 ` [PATCH 1/4] regulator: fix memory leak with repeated set_machine_constraints() Michał Mirosław
2020-11-13 0:16 ` [PATCH 4/4] regulator: workaround self-referent regulators Michał Mirosław
3 siblings, 0 replies; 5+ messages in thread
From: Michał Mirosław @ 2020-11-13 0:16 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown; +Cc: Ahmad Fatoum, linux-arm-kernel, linux-kernel
Help debugging the case when set_machine_constraints() needs to be
repeated.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/regulator/core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index bcd64ba21fb9..ad36f03d7ee6 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5296,6 +5296,8 @@ regulator_register(const struct regulator_desc *regulator_desc,
/* FIXME: this currently triggers a chicken-and-egg problem
* when creating -SUPPLY symlink in sysfs to a regulator
* that is just being created */
+ rdev_dbg(rdev, "will resolve supply early: %s\n",
+ rdev->supply_name);
ret = regulator_resolve_supply(rdev);
if (!ret)
ret = set_machine_constraints(rdev);
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 1/4] regulator: fix memory leak with repeated set_machine_constraints()
2020-11-13 0:16 [PATCH 0/4] regulator: debugging and fixing supply deps Michał Mirosław
2020-11-13 0:16 ` [PATCH 3/4] regulator: avoid resolve_supply() infinite recursion Michał Mirosław
2020-11-13 0:16 ` [PATCH 2/4] regulator: debug early supply resolving Michał Mirosław
@ 2020-11-13 0:16 ` Michał Mirosław
2020-11-13 0:16 ` [PATCH 4/4] regulator: workaround self-referent regulators Michał Mirosław
3 siblings, 0 replies; 5+ messages in thread
From: Michał Mirosław @ 2020-11-13 0:16 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown; +Cc: Ahmad Fatoum, linux-arm-kernel, linux-kernel
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: stable@vger.kernel.org
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 4/4] regulator: workaround self-referent regulators
2020-11-13 0:16 [PATCH 0/4] regulator: debugging and fixing supply deps Michał Mirosław
` (2 preceding siblings ...)
2020-11-13 0:16 ` [PATCH 1/4] regulator: fix memory leak with repeated set_machine_constraints() Michał Mirosław
@ 2020-11-13 0:16 ` Michał Mirosław
3 siblings, 0 replies; 5+ messages in thread
From: Michał Mirosław @ 2020-11-13 0:16 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown; +Cc: Ahmad Fatoum, linux-arm-kernel, linux-kernel
Workaround regulators whose supply name happens to be the same as its
own name. This fixes boards that used to work before the early supply
resolving was removed. The error message is left in place so that
offending drivers can be detected.
Fixes: aea6cb99703e ("regulator: resolve supply after creating regulator")
Cc: stable@vger.kernel.org
Reported-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/regulator/core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index ab922ed273f3..38ba579efe2b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1844,7 +1844,10 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
if (r == rdev) {
dev_err(dev, "Supply for %s (%s) resolved to itself\n",
rdev->desc->name, rdev->supply_name);
- return -EINVAL;
+ if (!have_full_constraints())
+ return -EINVAL;
+ r = dummy_regulator_rdev;
+ get_device(&r->dev);
}
/*
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread