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

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

[Michał Mirosław]

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).

(Series resent because of wrong arm-kernel ML address.)

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

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

[Michał Mirosław]

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).

(Series resent because of wrong arm-kernel ML address.)

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Michał Mirosław]

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

[PATCH RESEND 2/4] regulator: debug early supply resolving

[Michał Mirosław]

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

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

[Michał Mirosław]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RESEND 2/4] regulator: debug early supply resolving

[Michał Mirosław]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RESEND 4/4] regulator: workaround self-referent regulators

[Michał Mirosław]

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

[PATCH RESEND 3/4] regulator: avoid resolve_supply() infinite recursion

[Michał Mirosław]

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

[PATCH RESEND 3/4] regulator: avoid resolve_supply() infinite recursion

[Michał Mirosław]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RESEND 4/4] regulator: workaround self-referent regulators

[Michał Mirosław]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Ahmad Fatoum]

Hello Michał,

On 11/13/20 1:20 AM, Michał Mirosław wrote:
> 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).
> 
> (Series resent because of wrong arm-kernel ML address.)

lxa-mc1 (STM32MP1 board with STPMIC) now manages to boot again with following
new warning:

  stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Supply for VREF_DDR

So for the whole series,
Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de> # stpmic1

Thanks!
Ahmad

> 
> 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(-)
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

[Ahmad Fatoum]

Hello Michał,

On 11/13/20 1:20 AM, Michał Mirosław wrote:
> 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).
> 
> (Series resent because of wrong arm-kernel ML address.)

lxa-mc1 (STM32MP1 board with STPMIC) now manages to boot again with following
new warning:

  stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Supply for VREF_DDR

So for the whole series,
Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de> # stpmic1

Thanks!
Ahmad

> 
> 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(-)
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Ahmad Fatoum]

CC += linux-stm32 as other STM32MP1 boards are also affected.

On 11/13/20 12:19 PM, Ahmad Fatoum wrote:
> Hello Michał,
> 
> On 11/13/20 1:20 AM, Michał Mirosław wrote:
>> 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).
>>
>> (Series resent because of wrong arm-kernel ML address.)
> 
> lxa-mc1 (STM32MP1 board with STPMIC) now manages to boot again with following
> new warning:
> 
>   stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Supply for VREF_DDR
> 
> So for the whole series,
> Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de> # stpmic1
> 
> Thanks!
> Ahmad
> 
>>
>> 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(-)
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

[Ahmad Fatoum]

CC += linux-stm32 as other STM32MP1 boards are also affected.

On 11/13/20 12:19 PM, Ahmad Fatoum wrote:
> Hello Michał,
> 
> On 11/13/20 1:20 AM, Michał Mirosław wrote:
>> 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).
>>
>> (Series resent because of wrong arm-kernel ML address.)
> 
> lxa-mc1 (STM32MP1 board with STPMIC) now manages to boot again with following
> new warning:
> 
>   stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Supply for VREF_DDR
> 
> So for the whole series,
> Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de> # stpmic1
> 
> Thanks!
> Ahmad
> 
>>
>> 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(-)
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RESEND 2/4] regulator: debug early supply resolving

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 519 bytes --]

On Fri, Nov 13, 2020 at 01:20:27AM +0100, Michał Mirosław wrote:
> Help debugging the case when set_machine_constraints() needs to be
> repeated.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

Please when sending patches containing a mix of different kinds of
patches put any fixes at the start of the series before anything else,
including new features and trace additions like this one.  This makes it
much easier to send things as fixes without unneeded dependencies on the
new stuff.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH RESEND 2/4] regulator: debug early supply resolving

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 519 bytes --]

On Fri, Nov 13, 2020 at 01:20:27AM +0100, Michał Mirosław wrote:
> Help debugging the case when set_machine_constraints() needs to be
> repeated.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

Please when sending patches containing a mix of different kinds of
patches put any fixes at the start of the series before anything else,
including new features and trace additions like this one.  This makes it
much easier to send things as fixes without unneeded dependencies on the
new stuff.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Mark Brown]

On Fri, 13 Nov 2020 01:20:26 +0100, Michał Mirosław wrote:
> 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.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/3] regulator: fix memory leak with repeated set_machine_constraints()
      commit: 57a6ad482af256b2a13de14194fb8f67c1a65f10
[2/3] regulator: avoid resolve_supply() infinite recursion
      commit: 4b639e254d3d4f15ee4ff2b890a447204cfbeea9
[3/3] regulator: workaround self-referent regulators
      commit: f5c042b23f7429e5c2ac987b01a31c69059a978b

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Mark Brown]

On Fri, 13 Nov 2020 01:20:26 +0100, Michał Mirosław wrote:
> 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.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/1] regulator: debug early supply resolving
      commit: 0917c9db23accb8690d8f1987ada36eb5b1a5ac9

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel